Skip to content

simplify python requirements#261

Merged
bmw merged 2 commits intomainfrom
dedupe-python-reqs
Mar 7, 2026
Merged

simplify python requirements#261
bmw merged 2 commits intomainfrom
dedupe-python-reqs

Conversation

@bmw
Copy link
Member

@bmw bmw commented Mar 7, 2026

i was inspired to do this while working on #245

specifying both project.requires-python and tool.poetry.dependencies.python comes from #242 where we wanted to specify requires-python to get nice features from the PR description like:

  • Removed the version specific classifiers. Those are added by poetry-core automatically based on requires-python.
  • Removed tool.black.target-version since black is able to infer the value from project.requires-python.

the initial version of that PR removed both tool.poetry.dependencies.python and the upper bound on the python version. ohemorange correctly noted here that we need the upper bound on the python version for poetry's locking algorithm. this was fixed by following the suggestion from poetry's docs of specifying the python version in both places using slightly different values

poetry's docs make this recommendation for folks who want to avoid to avoid specifying an upper bound in their package's metadata, but i don't think that's something we need to do. as you can see by the metadata diff from the PR description, we previously were defining an upper bound:

...
-License: Apache-2.0
+License-Expression: Apache-2.0
License-File: LICENSE.txt
...
-Requires-Python: >=3.9.2,<4.0
+Requires-Python: >=3.9.2
...
-Classifier: License :: OSI Approved :: Apache Software License
...

to my knowledge, no one ever complained. not only does that approach duplicate information that we need to keep in sync, but project.requires-python and tool.poetry.dependencies.python take slightly different syntax with the ^3.10 caret requirement suggested in the comment above project.requires-python not being a valid value there

because of these downsides, this PR takes a slightly different approach and unifies everything in one place to hopefully make things a little simpler and cleaner for us

i don't think this PR requires two reviews

@bmw bmw requested a review from a team as a code owner March 7, 2026 00:28
@bmw bmw requested a review from ohemorange March 7, 2026 00:28
@bmw
Copy link
Member Author

bmw commented Mar 7, 2026

i just noticed that https://python-poetry.org/docs/dependency-specification/#caret-requirements doesn't exactly say that requires-python = "^3.10" is invalid, but it is. caret requirements are a poetry specific thing, the project section of pyproject.toml is a general python thing, and after doing a dummy release with this requires-python value, i end up with "Requires-Python: ^3.10" in the project metadata which i think would definitely break stuff

Copy link
Contributor

@ohemorange ohemorange left a comment

Choose a reason for hiding this comment

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

So...I'm going to approve, mostly because it's how we had it before and it's fine.

But I should mention that this is apparently a somewhat controversial topic, which is probably why poetry wrote the docs like that.

But that being said, poetry itself also specifies an upper bound.

In balance I think this is perfectly fine and makes our lives easier.

this PR also removes the part of the comment about changing the value back to the carat form, but given that you're working on dropping python 3.9, I also think that's fine.

@bmw
Copy link
Member Author

bmw commented Mar 7, 2026

But I should mention that this is apparently a somewhat controversial topic, which is probably why poetry wrote the docs like that.

yeah, i saw that too and started to get nerdsniped by it, but stopped and it ignored it for now for better or worse. my memory is poetry does get mad in some cases when we remove the upper bound entirely and until we have a strong reason to do this duplication, i personally prefer this approach

this PR also removes the part of the comment about changing the value back to the carat form, but given that you're working on dropping python 3.9, I also think that's fine.

i actually did this on purpose because ^3.10 isn't a valid requires-python value (although it is a valid tool.poetry.dependencies.python value). going forward i plan on just using the >=3.x,<4.0 form here

thanks for the review!

@bmw bmw merged commit e510d39 into main Mar 7, 2026
21 checks passed
@bmw bmw deleted the dedupe-python-reqs branch March 7, 2026 01:43
@bmw bmw mentioned this pull request Mar 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants