TST add tests for pyproject minimum dependency checks #32826

AnneBeyer · 2025-12-01T16:31:51Z

Reference Issues/PRs

What does this implement/fix? Explain your changes.

This adds a test using a minimal pyproject sample, to test that the tests actually work with upper bounds.
I had to restructure the code a bit in order to use a test string instead of the toml file.
Let me know if this is what you had in mind @lesteve.

Any other comments?

I noticed that there are also no tests testing the mismatch conditions of package versions. Adding this would be fairly straight forward now, but I'm not sure if these files are actually touched manually, and if this would make sense?

github-actions · 2025-12-01T16:32:51Z

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

_{Generated for commit: 0d18fb1. Link to the linter CI: here}

sklearn/tests/test_min_dependencies_readme.py

lesteve · 2025-12-03T15:19:48Z

sklearn/tests/test_min_dependencies_readme.py

-):
-    # tomllib is available in Python 3.11
-    tomllib = pytest.importorskip("tomllib")
+def extract_packages_and_pyproject_tags(dependencies):


Thanks for the PR! This seems to go in the direction I had in mind, I'll try to have a closer look at one point.

General comment: the more you can reduce the diff, the more this will make the reviewer's life (my life in this particular case) easier 😉. For example, it seems like you move global variables min_dependencies_tag_to_packages_without_version and pyproject_section_to_min_dependencies_tag to a function but this function is called once.

Maybe you have a good reason to do it and I would be curious to hear about it, but as a reviewer, I am expecting an "easy" PR to review and I have to take 5-10 minutes to understand just this part and that doesn't help keeping this PR in the "easy to review" category 😅.

I moved the global variables into a function in order to create them either from the min_dependencies or the test_dependencies, meaning the function is only called once, but the parameter passed from test_min_dependencies_pyproject_toml and test_check_pyproject_section_with_upper_bounds differs.
One could integrate the code into check_pyproject_sections, but since it is sort of a pre-processing step, I decided to keep it as an own function.

I tried to move the function to the same place as the global variables to make it easier to compare, but github still aligned the wrong parts, so I'll try to mark the changes below instead.

AnneBeyer · 2025-12-03T20:51:38Z

Thank you for the feedback!
I agree that the diff is really hard to read.

I had to fiddle around with the code a bit to make it easy to switch between real dependencies and a minimal test (because the code does not only check if the versions are the same, but also if the specified packages are the same (which makes sense), so I had to create matching minimal test samples for both min_dependencies and pyproject).

I tried to reduce the differences, and comparing this version to main in VSCode (using GitLens) actually doesn't look too bad, but here on GitHub it tries to align the wrong overlapping parts... 😞
I'll try to add some more explanations in the code, let me know if you can work with that.

AnneBeyer · 2025-12-03T21:19:20Z

sklearn/tests/test_min_dependencies_readme.py

-    tomllib = pytest.importorskip("tomllib")
+def extract_packages_and_pyproject_tags(dependencies):
+    min_depencies_tag_to_packages_without_version = defaultdict(list)
+    for package, (min_version, tags) in dependencies.items():


Here, it now iterates over the argument dependencies instead of the global imported dependent_packages

(I took the liberty of also renaming extras to tags, because it took my a while to figure out what "extras" are.)

AnneBeyer · 2025-12-03T21:23:25Z

sklearn/tests/test_min_dependencies_readme.py

+def check_pyproject_sections(pyproject_toml, min_dependencies):
+    packages, pyproject_tags = extract_packages_and_pyproject_tags(min_dependencies)
+
+    for pyproject_section, min_dependencies_tag in pyproject_tags.items():


I replaced the parametrization in the existing test with a for-loop here, to enable iterating over either the whole actual pyproject.toml or the given test subset.

AnneBeyer · 2025-12-03T21:24:52Z

sklearn/tests/test_min_dependencies_readme.py

+    for pyproject_section, min_dependencies_tag in pyproject_tags.items():
+        # NumPy is more complex because build-time (>=1.25) and run-time (>=1.19.5)
+        # requirement currently don't match
+        skip_version_check_for = ["numpy"] if min_dependencies_tag == "build" else []


Because the tags are only available at this stage, the definition of the skip packages has to happen here now.

AnneBeyer · 2025-12-03T21:27:01Z

sklearn/tests/test_min_dependencies_readme.py

+
+        pyproject_section_keys = pyproject_section.split(".")
+        info = pyproject_toml
+        # iterate through nested keys to get packages and version


I added a comment here because it took me a while to understand the following loop and what info actually contains at the end.

The remainder of this function is almost unchanged except for one extra indentation because of the loop above and one other change marked below.

AnneBeyer · 2025-12-03T21:39:02Z

sklearn/tests/test_min_dependencies_readme.py


-@pytest.mark.parametrize(
-    "pyproject_section, min_dependencies_tag",
-    pyproject_section_to_min_dependencies_tag.items(),


This parametrization is replaced by the for-loop in check_pyproject_sections(

AnneBeyer · 2025-12-03T21:41:07Z

sklearn/tests/test_min_dependencies_readme.py

+def test_min_dependencies_pyproject_toml():
+    """Check versions in pyproject.toml is consistent with _min_dependencies."""

    root_directory = Path(sklearn.__file__).parent.parent


Reading in the actual pyproject.toml file is now happening here, and the parsed version is passed to check_pyproject_sections (to enable passing a test version in the new test)

AnneBeyer · 2025-12-03T21:53:06Z

sklearn/tests/test_min_dependencies_readme.py

+
+        for package, version in pyproject_build_min_versions.items():
+            version = parse_version(version)
+            expected_min_version = parse_version(min_dependencies[package][0])


Here, the version is compared to the given min_dependencies, not the global imported dependent_packages (as in line 119 in the old version)

ogrisel

Please find a pass of review below.

ogrisel · 2025-12-10T09:52:53Z

sklearn/tests/test_min_dependencies_readme.py

-pyproject_section_to_min_dependencies_tag = {
-    "build-system.requires": "build",
-    "project.dependencies": "install",
+MIN_DEPENDENT_PACKAGES = {


I think we could use more explicit variable names to convey that those are not expected to match the currently used version numbers.

Suggested change

MIN_DEPENDENT_PACKAGES = {

EXAMPLE_MIN_DEPENDENT_PACKAGES = {

ogrisel · 2025-12-10T09:53:11Z

sklearn/tests/test_min_dependencies_readme.py

-    section = f"project.optional-dependencies.{tag}"
-    pyproject_section_to_min_dependencies_tag[section] = tag
+
+MATCHING_PYPROJECT_SECTIONS_WITH_UPPER_BOUND = """


Suggested change

MATCHING_PYPROJECT_SECTIONS_WITH_UPPER_BOUND = """

EXAMPLE_EXPECTED_PYPROJECT_SECTIONS_WITH_UPPER_BOUND = """

ogrisel · 2025-12-10T09:54:51Z

sklearn/tests/test_min_dependencies_readme.py

+install = ["joblib>=1.3.0,<2.0", "scipy==1.10.0"]
+[build-system]
+requires = ["scipy>=1.10.0,<1.19.0"]
+"""


Maybe add a similar example pyproject without the upper bounds to also test the check function against it?

ogrisel · 2025-12-10T10:02:23Z

sklearn/tests/test_min_dependencies_readme.py

-        min_dependencies_tag,
-        skip_version_check_for=skip_version_check_for,
-    )
+    check_pyproject_sections(pyproject_toml, MIN_DEPENDENT_PACKAGES)


Please also expand the test to also checks that a meaningful exception is raised when inconsistent arguments are passed to check_pyproject_sections.

…add one missing message and test)

add test for pyproject minimum dependency checks with upper bound

0f49c16

StefanieSenger added the No Changelog Needed label Dec 2, 2025

lesteve reviewed Dec 3, 2025

View reviewed changes

sklearn/tests/test_min_dependencies_readme.py Outdated Show resolved Hide resolved

lesteve reviewed Dec 3, 2025

View reviewed changes

AnneBeyer added 2 commits December 3, 2025 20:50

import tomllib globally

7f24b5c

reduce diff

3be1b3a

move function further down and rename variable to avoid confusion

0d18fb1

AnneBeyer marked this pull request as draft December 3, 2025 21:11

AnneBeyer commented Dec 3, 2025

View reviewed changes

AnneBeyer marked this pull request as ready for review December 3, 2025 21:58

ogrisel reviewed Dec 10, 2025

View reviewed changes

AnneBeyer added 2 commits December 10, 2025 11:50

rename test variables and add more matching and non-matching test cases

a51c1e6

extend non-matching tests to also check specific error messages (and …

8a1ce89

…add one missing message and test)

AnneBeyer changed the title ~~TST add test for pyproject minimum dependency checks with upper bound~~ TST add tests for pyproject minimum dependency checks Dec 10, 2025

adrinjalali assigned ogrisel, AnneBeyer and lesteve Dec 11, 2025

	MIN_DEPENDENT_PACKAGES = {
	EXAMPLE_MIN_DEPENDENT_PACKAGES = {

	MATCHING_PYPROJECT_SECTIONS_WITH_UPPER_BOUND = """
	EXAMPLE_EXPECTED_PYPROJECT_SECTIONS_WITH_UPPER_BOUND = """

Uh oh!

TST add tests for pyproject minimum dependency checks #32826

Are you sure you want to change the base?

TST add tests for pyproject minimum dependency checks #32826

Uh oh!

Conversation

AnneBeyer commented Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Any other comments?

Uh oh!

github-actions bot commented Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✔️ Linting Passed

Uh oh!

Uh oh!

lesteve Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AnneBeyer commented Dec 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AnneBeyer Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AnneBeyer Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ogrisel left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

AnneBeyer commented Dec 1, 2025 •

edited

Loading

github-actions bot commented Dec 1, 2025 •

edited

Loading

lesteve Dec 3, 2025 •

edited

Loading

AnneBeyer Dec 3, 2025 •

edited

Loading

AnneBeyer Dec 3, 2025 •

edited

Loading