-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[red-knot] Fallback to requires-python
if no python-version
is specified
#16028
Conversation
b2aeb58
to
dd75dc2
Compare
fad3730
to
a2f8e8e
Compare
|
||
tracing::debug!("Resolving requires-python constraint: `{requires_python}`"); | ||
|
||
let ranges = release_specifiers_to_ranges((&**requires_python).clone()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks to @konstin for pointing me toward the logic in uv. This matches what uv does. Thankfully, most of the work is done by the pep0440 crate.
7614b61
to
3dc7900
Compare
3dc7900
to
bd1f597
Compare
requires-python
if no python-version
is specified
4355d5d
to
521cc67
Compare
bd1f597
to
522bdb5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you. A few minor comments and one higher-level question.
522bdb5
to
d695c58
Compare
…r on requires python without lower bound
d695c58
to
8aae983
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
@@ -121,7 +119,7 @@ pub enum ResolveRequiresPythonError { | |||
TooLargeMajor(u64), | |||
#[error("The minor version `{0}` is larger than the maximum supported value 255")] | |||
TooLargeMinor(u64), | |||
#[error("value `{0}` does not contain a lower bound. Add a lower bound to indicate the minimum compatible Python version (e.g., `>=3.13`).")] | |||
#[error("value `{0}` does not contain a lower bound. Add a lower bound to indicate the minimum compatible Python version (e.g., `>=3.13`) or specify a version in `environment.python-version`.")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They might not have a tool.knot
table at all, right? This could be quite confusing if they aren't familiar with how to provide this red-knot-specific setting
#[error("value `{0}` does not contain a lower bound. Add a lower bound to indicate the minimum compatible Python version (e.g., `>=3.13`) or specify a version in `environment.python-version`.")] | |
#[error("value `{0}` does not contain a lower bound. Add a lower bound to indicate the minimum compatible Python version (e.g., `>=3.13`) or specify a version in `tool.knot.environment.python-version`.")] |
Summary
Add support for the
project.requires-python
field inpyproject.toml
files.Fall back to the resolved lower bound of
project.requires-python
if theenvironment.python-version
field isNone
(or more accurately, initializeenvironment.python-version with
requires-python`'s lower bound if left unspecified).UX design
There are two options on how we can handle the fallback to
requires-python
's lower bound:environment.python-version
if that field isNone
(Implemented in this PR)requires-python
constraint separately.There's no observed difference unless a user-level configuration (or any other inherited configuration is used). Let's discuss it on the given example
User configuration
Project configuration (
pyproject.toml
)The resolved version for 1. is 3.12 because the
requires-python
constraint precedence takes precedence over thepython-version
in the user configuration. 2. resolves to 3.10 because allpython-version
constraints take precedence before falling back torequires-python
.Ruff implements 1. It's also the easier to implement and it does seem intuitive to me that the more local
requires-python
constraint takes precedence.Test plan
Added CLI and unit tests.