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.19% +0.03%
==========================================
Files 40 40
Lines 3101 3101
Branches 680 680
==========================================
+ Hits 3075 3076 +1
+ Misses 15 14 -1
Partials 11 11 🚀 New features to boost your workflow:
|
DanielNoord
left a comment
There was a problem hiding this comment.
Thanks for the PR! Left some comments :)
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.
fe1d117 to
2738d63
Compare
No reason I can discern for it, just a matter of traditions I guess. PyCQA#2405 (comment)
No reason I can discern for it, just a matter of traditions I guess. PyCQA#2405 (comment)
4c39fc4 to
c676669
Compare
DanielNoord
left a comment
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 |
|
I installed it using and when using with |
|
@staticdev shall we just merge and release as is? |
No reason I can discern for it, just a matter of traditions I guess. PyCQA#2405 (comment)
c676669 to
80852d6
Compare
No reason I can discern for it, just a matter of traditions I guess. PyCQA#2405 (comment)
80852d6 to
c941d8d
Compare
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)
c941d8d to
be46cd4
Compare
|
I have rebased this branch on master and fixed the lint job. |
staticdev
left a comment
There was a problem hiding this comment.
That is fine, LGTM. Thanks @dvarrazzo and @DanielNoord
This PR contains the following updates: | Package | Change | Age | Confidence | |---|---|---|---| | [isort](https://github.com/PyCQA/isort) ([changelog](https://github.com/PyCQA/isort/releases)) | `<6.1.0,>=6.0.0` -> `<6.1.1,>=6.1.0` | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>PyCQA/isort (isort)</summary> ### [`v6.1.0`](https://github.com/PyCQA/isort/releases/tag/6.1.0) [Compare Source](PyCQA/isort@6.0.1...6.1.0) #### Changes - Update docs discussions channel ([#​2410](PyCQA/isort#2410)) [@​staticdev](https://github.com/staticdev) - Add python 3.14 classifier and badge ([#​2409](PyCQA/isort#2409)) [@​staticdev](https://github.com/staticdev) - Drop use of non-standard pkg\_resources API ([#​2405](PyCQA/isort#2405)) [@​dvarrazzo](https://github.com/dvarrazzo) - Use working isort version in pre-commit example ([#​2402](PyCQA/isort#2402)) [@​iainelder](https://github.com/iainelder) - fix typo in \_get\_files\_from\_dir\_cached test ([#​2392](PyCQA/isort#2392)) [@​tiltingpenguin](https://github.com/tiltingpenguin) - Resolve bandit warnings ([#​2379](PyCQA/isort#2379)) [@​kurtmckee](https://github.com/kurtmckee) - Add tox for cross-platform, parallel test suite execution ([#​2378](PyCQA/isort#2378)) [@​kurtmckee](https://github.com/kurtmckee) - Add Project URLs to PyPI Side Panel ([#​2387](PyCQA/isort#2387)) [@​guillermodotn](https://github.com/guillermodotn) - Fix typos ([#​2376](PyCQA/isort#2376)) [@​co63oc](https://github.com/co63oc) #### :construction\_worker: Continuous Integration - Add make bash scripts portable ([#​2377](PyCQA/isort#2377)) [@​staticdev](https://github.com/staticdev) #### 📦 Dependencies - Bump actions/checkout from 4 to 5 in the github-actions group ([#​2406](PyCQA/isort#2406)) @​[dependabot\[bot\]](https://github.com/apps/dependabot) - Bump astral-sh/setup-uv from 5 to 6 in the github-actions group ([#​2395](PyCQA/isort#2395)) @​[dependabot\[bot\]](https://github.com/apps/dependabot) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MS4xMzIuNSIsInVwZGF0ZWRJblZlciI6IjQxLjEzMi41IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJ0eXBlL2RlcGVuZGVuY2llcyJdfQ==--> Reviewed-on: https://git.tainton.uk/repos/roboluke/pulls/380 Co-authored-by: Renovate [BOT] <renovate-bot@git.tainton.uk> Co-committed-by: Renovate [BOT] <renovate-bot@git.tainton.uk>
This PR contains the following updates: | Package | Change | Age | Confidence | |---|---|---|---| | [isort](https://github.com/PyCQA/isort) ([changelog](https://github.com/PyCQA/isort/releases)) | `<6.1.0,>=6.0.0` -> `<6.1.1,>=6.1.0` | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>PyCQA/isort (isort)</summary> ### [`v6.1.0`](https://github.com/PyCQA/isort/releases/tag/6.1.0) [Compare Source](PyCQA/isort@6.0.1...6.1.0) #### Changes - Update docs discussions channel ([#​2410](PyCQA/isort#2410)) [@​staticdev](https://github.com/staticdev) - Add python 3.14 classifier and badge ([#​2409](PyCQA/isort#2409)) [@​staticdev](https://github.com/staticdev) - Drop use of non-standard pkg\_resources API ([#​2405](PyCQA/isort#2405)) [@​dvarrazzo](https://github.com/dvarrazzo) - Use working isort version in pre-commit example ([#​2402](PyCQA/isort#2402)) [@​iainelder](https://github.com/iainelder) - fix typo in \_get\_files\_from\_dir\_cached test ([#​2392](PyCQA/isort#2392)) [@​tiltingpenguin](https://github.com/tiltingpenguin) - Resolve bandit warnings ([#​2379](PyCQA/isort#2379)) [@​kurtmckee](https://github.com/kurtmckee) - Add tox for cross-platform, parallel test suite execution ([#​2378](PyCQA/isort#2378)) [@​kurtmckee](https://github.com/kurtmckee) - Add Project URLs to PyPI Side Panel ([#​2387](PyCQA/isort#2387)) [@​guillermodotn](https://github.com/guillermodotn) - Fix typos ([#​2376](PyCQA/isort#2376)) [@​co63oc](https://github.com/co63oc) #### :construction\_worker: Continuous Integration - Add make bash scripts portable ([#​2377](PyCQA/isort#2377)) [@​staticdev](https://github.com/staticdev) #### 📦 Dependencies - Bump actions/checkout from 4 to 5 in the github-actions group ([#​2406](PyCQA/isort#2406)) @​[dependabot\[bot\]](https://github.com/apps/dependabot) - Bump astral-sh/setup-uv from 5 to 6 in the github-actions group ([#​2395](PyCQA/isort#2395)) @​[dependabot\[bot\]](https://github.com/apps/dependabot) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MS4xMzIuNSIsInVwZGF0ZWRJblZlciI6IjQxLjEzMi41IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJ0eXBlL2RlcGVuZGVuY2llcyJdfQ==--> Reviewed-on: https://git.tainton.uk/repos/epage/pulls/164 Co-authored-by: Renovate [BOT] <renovate-bot@git.tainton.uk> Co-committed-by: Renovate [BOT] <renovate-bot@git.tainton.uk>
This PR contains the following updates: | Package | Change | Age | Confidence | |---|---|---|---| | [isort](https://github.com/PyCQA/isort) ([changelog](https://github.com/PyCQA/isort/releases)) | `<6.1.0,>=6.0.0` -> `<6.1.1,>=6.1.0` | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>PyCQA/isort (isort)</summary> ### [`v6.1.0`](https://github.com/PyCQA/isort/releases/tag/6.1.0) [Compare Source](PyCQA/isort@6.0.1...6.1.0) #### Changes - Update docs discussions channel ([#​2410](PyCQA/isort#2410)) [@​staticdev](https://github.com/staticdev) - Add python 3.14 classifier and badge ([#​2409](PyCQA/isort#2409)) [@​staticdev](https://github.com/staticdev) - Drop use of non-standard pkg\_resources API ([#​2405](PyCQA/isort#2405)) [@​dvarrazzo](https://github.com/dvarrazzo) - Use working isort version in pre-commit example ([#​2402](PyCQA/isort#2402)) [@​iainelder](https://github.com/iainelder) - fix typo in \_get\_files\_from\_dir\_cached test ([#​2392](PyCQA/isort#2392)) [@​tiltingpenguin](https://github.com/tiltingpenguin) - Resolve bandit warnings ([#​2379](PyCQA/isort#2379)) [@​kurtmckee](https://github.com/kurtmckee) - Add tox for cross-platform, parallel test suite execution ([#​2378](PyCQA/isort#2378)) [@​kurtmckee](https://github.com/kurtmckee) - Add Project URLs to PyPI Side Panel ([#​2387](PyCQA/isort#2387)) [@​guillermodotn](https://github.com/guillermodotn) - Fix typos ([#​2376](PyCQA/isort#2376)) [@​co63oc](https://github.com/co63oc) #### :construction\_worker: Continuous Integration - Add make bash scripts portable ([#​2377](PyCQA/isort#2377)) [@​staticdev](https://github.com/staticdev) #### 📦 Dependencies - Bump actions/checkout from 4 to 5 in the github-actions group ([#​2406](PyCQA/isort#2406)) @​[dependabot\[bot\]](https://github.com/apps/dependabot) - Bump astral-sh/setup-uv from 5 to 6 in the github-actions group ([#​2395](PyCQA/isort#2395)) @​[dependabot\[bot\]](https://github.com/apps/dependabot) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MS4xMzIuNSIsInVwZGF0ZWRJblZlciI6IjQxLjEzMi41IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJ0eXBlL2RlcGVuZGVuY2llcyJdfQ==--> Reviewed-on: https://git.tainton.uk/repos/webexmemebot/pulls/510 Co-authored-by: Renovate [BOT] <renovate-bot@git.tainton.uk> Co-committed-by: Renovate [BOT] <renovate-bot@git.tainton.uk>
Use the canonical
importlib.metadata.entry_pointsinstead.Fix #2404