Drop use of non-standard pkg_resources API #2405
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2405 +/- ##
==========================================
- Coverage 99.16% 99.12% -0.04%
==========================================
Files 40 40
Lines 3101 3101
Branches 680 681 +1
==========================================
- Hits 3075 3074 -1
Misses 15 15
- Partials 11 12 +1 🚀 New features to boost your workflow:
|
isort/settings.py
Outdated
| from .wrap_modes import WrapModes | ||
| from .wrap_modes import from_string as wrap_mode_from_string | ||
|
|
||
| if sys.version_info < (3, 10): |
There was a problem hiding this comment.
Can we keep these imports lazy like it was before?
There was a problem hiding this comment.
To avoid adding an if every time there was an import pkg_resources I'd have a _pkg_resources internal module exposing entry_points.
What is the rationale?
There was a problem hiding this comment.
You could also do this with a cached function in this module that returns the import I think.
Originally somebody thought it better to make this lazy. I don't see a good reason for why we would want to change this so I'd prefer to keep it the same.
There was a problem hiding this comment.
Well, when the code was importing from pkg_resources, which is a non-standard module teleported in Python virtualenv thanks to obscure forces, I can somewhat understand why the import was lazy: if it suddenly started failing (like now, in Python 3.14) I assume the thought was at least it won't fail for everyone on import but just for people who use this less used feature.
Now that the code is importing either a stdlib function or a function from a package marked as dependency, I don't see a good reason for failing.
AFAICS the package was first used in 2d76984, then some code following a similar pattern was added in b40e584, then again in 4cb72fa. I assume the code was copied over and modified, including the lazy-loading pattern. Maybe @timothycrosley might explain why those import are lazy?
Anyways, the last commit in this MR restores the lazy import behaviour. I don't think it is needed, especially if you have unit tests testing that the package works, but if you want it this way, it's there.
dvarrazzo
force-pushed
the
fix/drop-pkg-resources
branch
from
fe1d117 to
2738d63
Compare
No reason I can discern for it, just a matter of traditions I guess. PyCQA#2405 (comment)
Use the canonical `importlib.metadata.entry_points` instead, and a well defined fallback for older Python versions. Fix PyCQA#2404
No reason I can discern for it, just a matter of traditions I guess. PyCQA#2405 (comment)
dvarrazzo
force-pushed
the
fix/drop-pkg-resources
branch
from
4c39fc4 to
c676669
Compare
There was a problem hiding this comment.
@staticdev Could you review this as well? I'm wondering what you think of the lazy loading.
There was a problem hiding this comment.
I would actually remove support for python below 3.10., as 3.9 is pretty much eol: https://endoflife.date/python this actually will simplify the code and the Pr.
I think we can keep the lazy loading like it is right now.
|
Seems like a lot more work to do proper clean up of 3.9? I'd say if this work we could at least just merge and release this and unblock Afterwards we can worry about cleaning up for |
Use the canonical
importlib.metadata.entry_pointsinstead.Fix #2404