Giter Club home page Giter Club logo

Comments (5)

akihironitta avatar akihironitta commented on May 13, 2024

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

from utilities.

Borda avatar Borda commented on May 13, 2024

@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 :)

from utilities.

marcinwrochna avatar marcinwrochna commented on May 13, 2024

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.

from utilities.

Borda avatar Borda commented on May 13, 2024

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 🐿️

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.

from utilities.

Borda avatar Borda commented on May 13, 2024

Other listed issues - Lightning-AI/pytorch-lightning#16756

from utilities.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.