Simplify pep440 -> version ranges conversion #8683
Conversation
With #8669 as preparation, we can remove what used to be the `PubGrubSpecifier` and instead implement a direct conversion from `VersionSpecifiers` to `Range<Version>`. Thanks to #8669 (comment), this conversion is infallible, removing a lot of fallibility downstream. The conversion takes an owned value, to signal that we consume the value. Previously, the function had cloned the version internally.
konstin
force-pushed
the
konsti/version-ranges-3
branch
from
7a3dbbe to
c30a666
Compare
| fn from(specifiers: VersionSpecifiers) -> Self { | ||
| let mut range = Ranges::full(); | ||
| for specifier in specifiers { | ||
| range = range.intersection(&Self::from(specifier)); |
There was a problem hiding this comment.
At first, I thought this might have been allocating a new Ranges<Version> only to throw it away, but I think Ranges::from(specifier) in this case always creates a Ranges<Version> with one range? (Possibly two?) If so, and I believe since it uses smallvec internally, there's no alloc happening.
I don't have any suggestions for the immediate term, but if this ever showed up on a profile, there might be some room for micro-optimization here. (Perhaps requiring API changes to version-ranges. Not sure.)
There was a problem hiding this comment.
Also guessing that this is mostly moved code and not newly written.
There was a problem hiding this comment.
Agreed, keeping it since it's the same overhead as it was before the refactoring.
| @@ -11,7 +11,7 @@ use crate::RequiresPython; | |||
| #[test] | |||
| fn requires_python_included() { | |||
| let version_specifiers = VersionSpecifiers::from_str("==3.10.*").unwrap(); | |||
| let requires_python = RequiresPython::from_specifiers(&version_specifiers).unwrap(); | |||
| @@ -362,7 +362,7 @@ async fn init_project( | |||
| } | |||
| ref | |||
| python_request @ PythonRequest::Version(VersionRequest::Range(ref specifiers, _)) => { | |||
| let requires_python = RequiresPython::from_specifiers(specifiers)?; | |||
With #8669 as preparation, we can remove what used to be the
PubGrubSpecifierand instead implement a direct conversion fromVersionSpecifierstoRange<Version>. Thanks to #8669 (comment), this conversion is infallible, removing a lot of fallibility downstream.The conversion takes an owned value, to signal that we consume the value. Previously, the function had cloned the version internally.