Skip to content

fix(scripts): replace pkg_resources in collect_info with importlib.metadata - #2355

Open
Ludwig J. Marx (LudwigJMarx) wants to merge 2 commits into
microsoft:mainfrom
LudwigJMarx:fix/collect-info-pkg-resources
Open

Ludwig J. Marx (LudwigJMarx) wants to merge 2 commits into
microsoft:mainfrom
LudwigJMarx:fix/collect-info-pkg-resources

Conversation

@LudwigJMarx

Copy link
Copy Markdown

Description

scripts/collect_info.py used pkg_resources, which setuptools stopped
shipping with 82.0.0, and it treated every name in REQUIRED as installed.
Switched to importlib.metadata and made a missing package print a line
instead 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.metadata has been in the standard library since Python 3.8 and
requires-python is >=3.8.0, so this adds no dependency.

Two causes, and the second is only reachable once the first is fixed:
import pkg_resources stops the script before it starts, and REQUIRED lists
wheel and cython, which sit in build-system.requires rather than in
dependencies. Both stand in the way of the same outcome, so they are handled
together rather than in two PRs.

Rejected: dropping wheel and cython from REQUIRED. For a diagnostic
script 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?

  • Pass the test by running: pytest qlib/tests/test_all_pipeline.py under upper directory of qlib.
  • If you are adding a new feature, test on your own test scripts.

New file tests/test_collect_info.py. The result must not depend on whichever
setuptools the machine happens to hold, so the test runs the script in a
subprocess behind a meta_path finder that blocks pkg_resources. That keeps
it red on any setuptools version while the import is in the file. The two
commits carry the evidence: red on 8c7e8698, green on 38a30864.

Before the fix:

FAILED test_collect_info.py::TestCollectInfo::test_runs_without_pkg_resources
FAILED test_collect_info.py::TestCollectInfo::test_reports_a_package_that_is_not_installed
E   AssertionError: 1 != 0 : collect_info.py all failed:
E     File "collect_info.py", line 5, in <module>
E       import pkg_resources
E   ImportError: pkg_resources is not available
2 failed in 2.05s

After the fix:

2 passed in 1.61s

And by hand, the way the template asks for it
(cd scripts && python collect_info.py all), exit 0:

Qlib version: 0.9.8.dev34
setuptools==84.0.0
wheel==0.48.0
cython: not installed
pyyaml==6.0.3
...
setuptools-scm==10.2.3

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).version against
importlib.metadata.version(x) over seven packages including ruamel.yaml,
python-redis-lock, pydantic-settings and setuptools-scm: zero
differences.

Rest of the suite, same command as CI (cd tests && pytest . -m "not slow"):

result
before, untouched tree 71 passed, 1 skipped, 10 deselected, 79 subtests
after 73 passed, 1 skipped, 10 deselected, 79 subtests

The two extra are the new tests.

black -l 120 --check clean, pylint 10.00/10 on scripts/collect_info.py.

Environment: Python 3.12.13, setuptools 84.0.0, MacOS 26.6.2 arm64, commit
be72549.

Types of changes

  • Fix bugs
  • Add new feature
  • Update documentation

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant