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

Always use release-only comparisons for requires-python #4794

Merged
merged 1 commit into from
Jul 4, 2024

Conversation

charliermarsh
Copy link
Member

@charliermarsh charliermarsh commented Jul 3, 2024

Summary

There are a few ideas at play here:

  1. pip always strips versions to the release when evaluating against a Requires-Python, so we now do the same. That means, e.g., using 3.13.0b0 will be accepted by a project with Requires-Python: >= 3.13, which does not adhere to PEP 440 semantics but is somewhat intuitive.
  2. Because we know we'll only be evaluating against release-only versions, we can use different semantics in PubGrub that let us collapse ranges. For example, python_version >= '3.10' or python_version < '3.10' can be collapsed to the truthy marker.

Closes #4714.
Closes #4272.
Closes #4719.

@charliermarsh charliermarsh added the bug Something isn't working label Jul 3, 2024
@charliermarsh charliermarsh force-pushed the charlie/requires-python-upper-bound branch from 4cfb8a2 to 7c152c9 Compare July 3, 2024 23:54
@charliermarsh
Copy link
Member Author

Gotta fix the snapshots, which is just about how we display those versions.

#[inline]
#[must_use]
pub fn only_release(&self) -> Self {
Self::new(self.release().iter().copied())
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth trying to avoid a copy here?

Copy link
Member

@konstin konstin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add this to the documentation, otherwise the requires-python semantics are indecipherable after reading our prerelease version handling section.

@@ -148,3 +148,101 @@ impl TryFrom<&VersionSpecifier> for PubGrubSpecifier {
Ok(Self(ranges))
}
}

impl PubGrubSpecifier {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we merge the two impl PubGrubSpecifier blocks?

@charliermarsh
Copy link
Member Author

Sounds good. I'll add some more documentation.

@charliermarsh charliermarsh force-pushed the charlie/requires-python-upper-bound branch 2 times, most recently from ca57771 to 309d708 Compare July 4, 2024 18:19
@charliermarsh charliermarsh force-pushed the charlie/requires-python-upper-bound branch from 309d708 to 416e17c Compare July 4, 2024 19:59
@charliermarsh charliermarsh enabled auto-merge (squash) July 4, 2024 19:59
@charliermarsh charliermarsh merged commit b588054 into main Jul 4, 2024
47 checks passed
@charliermarsh charliermarsh deleted the charlie/requires-python-upper-bound branch July 4, 2024 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants