Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inference of target versions from requires-python config is incorrect. #3581

Open
manueljacob opened this issue Feb 24, 2023 · 2 comments · May be fixed by #3583
Open

Inference of target versions from requires-python config is incorrect. #3581

manueljacob opened this issue Feb 24, 2023 · 2 comments · May be fixed by #3583
Labels
T: bug Something isn't working

Comments

@manueljacob
Copy link

manueljacob commented Feb 24, 2023

#3219 introduced inference of the target versions for black from the requires-python project metadata in pyproject.toml. However, it was not implemented correctly.

The rules for the comparison operators in requires-python is documented in PEP 440.

For example, >3.7,<3.10 currently results in the inference of ["py38", "py39"]. However, 3.7.1 is in >3.7,<3.10, so "py37" should be included as well.

>3.7.0,<3.10 results in the inference of ["py37", "py38", "py39"], which is correct. However, >3.7.0,<3.10 is equivalent to >3.7,<3.10, so they should give the same results.

I re-implemented the inference code. I will open a pull request as soon as I clarified and commented the code.

@manueljacob
Copy link
Author

Currently, we treat empty requires-python metadata as if it was not present. But technically, it’s a version specifier with no clauses, so it contains all Python versions. On one hand, if we want to respect the configuration and be consistent, we should treat empty requires-python metadata as “all Python versions”. On the other hand, empty requires-python metadata is very non-explicit, so falling back to per-file auto-detection doesn’t seem too wrong, either. What do you think?

@stinodego
Copy link
Contributor

stinodego commented Feb 26, 2023

You're right, >3.7 should be treated as >3.7.0, so it should include py37. That's an oversight on my part. Great that you're working on a fix 👍 should be a small adjustment.

manueljacob added a commit to manueljacob/black that referenced this issue Feb 27, 2023
Fixes psf#3581.

The old algorithm checked, for each target version 3.X, whether 3.X (which is roughly the same as 3.X.0) is contained in a version specifier that was modified from the requires-python project metadata to be less strict regarding patch versions. One problem of it was that, for a requires-python value of ">3.X" (which means the same as ">3.X.0"), it concluded that target version 3.X is not supported although Python versions >= 3.X.1 are included in the version specifier. I found the old approach hard to reason about and hard to fix.

To correctly check whether a target version 3.X is supported, the algorithm must check if 3.X.* overlaps the version specifier in the requires-python project metadata. Checking only specific versions (like 3.X.0) is not sufficient in general. The `packaging` library, which implements the logic for (PEP440-compatible) versions and version specifiers, doesn’t implement checking for overlap of two version specifiers, however.

The new algorithm works by converting the version specifiers to interval sets, which are then checked for overlap.
manueljacob added a commit to manueljacob/black that referenced this issue Feb 27, 2023
Fixes psf#3581.

The old algorithm checked, for each target version 3.X, whether 3.X (which is roughly the same as 3.X.0) is contained in a version specifier that was modified from the requires-python project metadata to be less strict regarding patch versions. One problem of it was that, for a requires-python value of ">3.X" (which means the same as ">3.X.0"), it concluded that target version 3.X is not supported although Python versions >= 3.X.1 are included in the version specifier. I found the old approach hard to reason about and hard to fix.

To correctly check whether a target version 3.X is supported, the algorithm must check whether 3.X.* overlaps the version specifier in the requires-python project metadata. Checking only specific versions (like 3.X.0) is not sufficient in general. The `packaging` library, which implements the logic for (PEP440-compatible) versions and version specifiers, doesn’t implement checking for overlap of two version specifiers, however.

The new algorithm works by converting the version specifiers to interval sets, which are then checked for overlap.
@manueljacob manueljacob linked a pull request Feb 27, 2023 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants