utilities icon indicating copy to clipboard operation
utilities copied to clipboard

`pkg_resources` is deprecated

Open marcinwrochna opened this issue 2 years ago • 5 comments

Bug

src/lightning_utilities/core/imports.py imports pkg_resources, which is deprecated (since at least 2020) in favor of importlib.resources, importlib.metadata, and their backports (importlib_resources, importlib_metadata). See here.

To Reproduce

PYTHONWARNINGS=default::DeprecationWarning python -c 'import torchmetrics'

(or import lightning_utilities)

marcinwrochna avatar Mar 07 '23 16:03 marcinwrochna

@marcinwrochna Thanks for reporting this issue! Feel free to send a PR if you're interested :)

akihironitta avatar Mar 08 '23 10:03 akihironitta

@marcinwrochna Thanks for reporting this issue! Feel free to send a PR if you're interested :)

Yeas that would be a nice and valuable contribution if you have a time to do it :)

Borda avatar Mar 16 '23 06:03 Borda

Sorry, this requires some understanding of all the requirements in lightning projects and of python packaging in general – I have neither, so I'll just say what I found by looking into it.

Three files currently use pkg_resources. In all cases, they cannot be directly replaced with importlib, because there is no parse_requirements, Requirements, nor require() there. I believe the intention here is that one shouldn't try to reimplement distribution-package requirement checks outside of pip.

  1. Your setup.py uses it, as far as I understand, only to check that requirement strings would be parsed correctly (or maybe to normalize them?). It can be left as is, because setup.py won't be imported by users. But it would probably be better to avoid arbitrary code in package requirement definitions, and instead just test whether the plaintext requirements work as expected (at least with pip install). This makes requirements discoverability possible, and doesn't assume that pip parses it exactly like your version of the deprecated pkg_resources package does. Alternatively, you could probably use packaging.requirements instead, as you already do in other places.

  2. You use it in load_requirements in src/lightning_utilities/install/requirements.py. But a) I don't know why all these workarounds are needed, maybe the requirement strings could be fixed instead, maybe it's because you use pkg_resources.parse_requirements() in the first place and maybe it doesn't match with pip's behaviour (see 1.)? Moreover, b) you never use that definition? Instead you use another implementation in https://github.com/Lightning-AI/lightning/blob/master/src/lightning/app/utilities/packaging/build_config.py and yet another in https://github.com/Lightning-AI/lightning/blob/master/.actions/assistant.py . I'm confused.

  3. You use it in src/lightning_utilities/core/imports.py for compare_version and RequirementCache. The usage in compare_version can easily be replaced with importlib. RequirementCache however uses require(), which isn't available elsewhere. But do you really need it? It's unclear even whether RequirementCache is supposed to check distribution packages or import packages. It seems you only use it to check simple name>=version requirements (not the various complicated requirement strings) and also only in situations where you'd import the module anyway. Maybe you could just try-import the appropriate module and check it's __version__? It seems you wanted to give a hint like "Use pip install foo", but you could just provide that hint as an additional argument in situations where the distribution package name isn't obvious? By the way, maybe instead of storing RequirementCache instances as global variables you should make it a function decorated with @lru_cache.

marcinwrochna avatar Mar 25 '23 15:03 marcinwrochna

I believe the intention here is that one shouldn't try to reimplement distribution-package requirement checks outside of pip

I found this quite calling as setup.py is called when installing on a blank environment, so it may need to somehow wrap the build class and so have local import of the package inside :chipmunk:

I would see two steps:

  1. replace pkg_resources within this package
  2. find a pattern/way how to use these functions in another setup on blank env.

Borda avatar Mar 28 '23 17:03 Borda

Other listed issues - https://github.com/Lightning-AI/lightning/issues/16756

Borda avatar Oct 07 '23 02:10 Borda