Infer python dist ics from pyproject#23495
Conversation
ndellosa95
left a comment
ndellosa95
left a comment
There was a problem hiding this comment.
I have a few nits but looks good overall and thanks for handling this!
| value: InterpreterConstraints | ||
|
|
||
|
|
||
| def _pyproject_requires_python(contents: DigestContents, address: Address) -> str | None: |
There was a problem hiding this comment.
Can we change the signature of this function to just take file_content: bytes and move the digest contents unboxing and exception handling to the rule? This makes it easier to unit test the specific toml parsing we want to do.
|
|
||
|
|
||
| @rule | ||
| async def resolve_python_distribution_effective_interpreter_constraints( |
There was a problem hiding this comment.
Could we cut out the wrapper types and go right from PythonDistributionInterpreterConstraintsField -> InterpreterConstraints?
| if requires_python is None: | ||
| return None | ||
| if not isinstance(requires_python, str): | ||
| raise InvalidFieldException( |
There was a problem hiding this comment.
We typically use InvalidFieldException for invalid Pants fields. I think there's a separate exception we typically use here, or maybe just ValueError?
|
Thanks for the contribution. We've just branched for 2.33.x, so merging this pull request now will come out in 2.34.x, please move the release notes updates to docs/notes/2.34.x.md if that's appropriate. |
2222f03 to
f56d292
Compare
|
Thank you for taking the time to review these changes @ndellosa95 I have addressed your comments, but please do let me know if any follow-up change is required. Also, I have moved the release notes to 2.34 @cburroughs |
3 participants
A
python_distributiontarget now infers itsinterpreter_constraintsfrom the[project].requires-pythonvalue in apyproject.tomlin the same directory, when the field is not set explicitly. An explicitinterpreter_constraintsfield still takes precedence, and targets without a siblingpyproject.toml(orrequires-python) continue to fall back to[python].interpreter_constraints(#23417).Happy to take feedback and add / remove changes that maintainers suggest.