fix(scripts): replace pkg_resources in collect_info with importlib.metadata - #2355
Open
Ludwig J. Marx (LudwigJMarx) wants to merge 2 commits into
Open
Ludwig J. Marx (LudwigJMarx) wants to merge 2 commits into
Ludwig J. Marx (LudwigJMarx) wants to merge 2 commits into
Conversation
The bug report template points users at collect_info.py for their environment details, and nothing held the script to actually running. Whether pkg_resources resolves depends on the installed setuptools, so the test would otherwise pass or fail by accident. It runs the script in a subprocess behind a meta_path finder that blocks pkg_resources, which keeps it red on any setuptools version while the import is in the file. The second case carries a skipIf: where cython is installed, a missing package from that list cannot be shown. Both fail on this commit. Refs microsoft#2354
…tadata setuptools stopped shipping pkg_resources with 82.0.0. Measured rather than taken from the changelog: 80.9.0 ships it, 81.0.0 still ships it behind a deprecation warning, 82.0.0 does not. Since Python 3.12 venv no longer seeds setuptools either, so the module can be absent regardless. importlib.metadata has been in the standard library since 3.8 and requires-python is >=3.8.0, so this adds no dependency. Checked on setuptools 81, where both APIs exist, that they report the same versions across the awkward names: ruamel.yaml, python-redis-lock, pydantic-settings, setuptools-scm. No differences. The second cause sits behind the first: REQUIRED lists wheel and cython, which are in build-system.requires rather than dependencies, so a plain install does not have them and the script died part way through its output. A missing package now prints a line. Dropping those two names from the list was rejected. For a diagnostic script the useful answer is that a package is missing, not an abort. Refs microsoft#2354
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
scripts/collect_info.pyusedpkg_resources, which setuptools stoppedshipping with 82.0.0, and it treated every name in
REQUIREDas installed.Switched to
importlib.metadataand made a missing package print a lineinstead of raising.
Motivation and Context
Refs #2354.
The bug report template points users at this script for their environment
details, so it fails exactly when someone is trying to file a report.
importlib.metadatahas been in the standard library since Python 3.8 andrequires-pythonis>=3.8.0, so this adds no dependency.Two causes, and the second is only reachable once the first is fixed:
import pkg_resourcesstops the script before it starts, andREQUIREDlistswheelandcython, which sit inbuild-system.requiresrather than independencies. Both stand in the way of the same outcome, so they are handledtogether rather than in two PRs.
Rejected: dropping
wheelandcythonfromREQUIRED. For a diagnosticscript the useful answer is that a package is missing, not an abort, and the
same handling covers later edits to that list.
How Has This Been Tested?
pytest qlib/tests/test_all_pipeline.pyunder upper directory ofqlib.New file
tests/test_collect_info.py. The result must not depend on whicheversetuptools the machine happens to hold, so the test runs the script in a
subprocess behind a
meta_pathfinder that blockspkg_resources. That keepsit red on any setuptools version while the import is in the file. The two
commits carry the evidence: red on
8c7e8698, green on38a30864.Before the fix:
After the fix:
And by hand, the way the template asks for it
(
cd scripts && python collect_info.py all), exit 0:Counter-check that the replacement reports the same values: in a venv on
setuptools 81, where both APIs still exist,
pkg_resources.get_distribution(x).versionagainstimportlib.metadata.version(x)over seven packages includingruamel.yaml,python-redis-lock,pydantic-settingsandsetuptools-scm: zerodifferences.
Rest of the suite, same command as CI (
cd tests && pytest . -m "not slow"):The two extra are the new tests.
black -l 120 --checkclean,pylint10.00/10 onscripts/collect_info.py.Environment: Python 3.12.13, setuptools 84.0.0, MacOS 26.6.2 arm64, commit
be72549.
Types of changes