-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Officially drop Python 3.8 support #13386
Conversation
This is part of gradually dropping support, cf. python#12112
This also removes the check that the |
lib/ts_utils/metadata.py
Outdated
assert requires_python != oldest_supported_python_specifier, f'requires_python="{requires_python}" is redundant' | ||
# Check minimum Python version is not less than the oldest version of Python supported by typeshed | ||
assert oldest_supported_python_specifier.contains( | ||
requires_python.version | ||
), f"'requires_python' contains versions lower than typeshed's oldest supported Python ({oldest_supported_python})" |
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.
I don't mind moving the assertions if you feel strongly, but I'd prefer not to just delete it. They seem useful to me; I think we should fix the assertion failures that appeared on previous commits in this PR.
TBH I'm not really sure why we need to move the assertions at all, though; the docstring of this function already says that this function does some basic validation as well as parsing.
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.
The problem is that the assertions fail, since oldest_supported_python
is now higher than what some requires_python
fields in some METADATA.toml
files say. We could remove those fields now, but currently the plan in #12112 says to only remove them in April. And I think this makes sense as that is safer. And those extra requires_python
fields are not harmful, having them unnecessarily is mostly a cosmetic thing.
That said, I don't feel super strongly about keeping the requires_python
fields, but I think keeping them is the better option than keeping a check for a cosmetic warning.
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.
Ah, thanks, this makes more sense to me now!
And I think this makes sense as that is safer.
I'm actually not sure about that. The effect of removing the requires-python
fields now would be that the stub-uploader will immediately push new versions of all those packages that cannot be installed using Python 3.8. But a third-party stub that doesn't have a requires-python
field in its METADATA.toml
will also not be installable with Python 3.8 in its next update pushed to PyPI, due to the global oldest_supported_python
field in our pyproject.toml file; the only real difference is that there won't be an immediate release of the package that drops support for Python 3.8. And for a package that still has requires-python = ">= 3.8"
in its METADATA.toml
file, that opens up the possibility that we might start accidentally using some syntax in that stubs package which is invalid on Python 3.8, even though the stubs package still declares that it can be installed on Python 3.8. We wouldn't notice, because we no longer run Python 3.8 in CI.
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.
Makes sense, I'll restore the checks and instead remove requires_python
. (And update the issue.)
This reverts commit e3d5f49.
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!
This is part of gradually dropping support, cf. #12112