-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Enable and enforce modern type annotations across the codebase #13371
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
Conversation
(FWIW, this can wait until the dust has settled on 25.1.x bugfixes -- I'll update this as necessary) |
Previously there has been trepidation about using I know PEP 749 is meant to clear that up, but it's not clear to me what the state of that PEP is and what it's currently proposing (the discussion evolved a lot). |
I think it's reasonably safe for us to rely on this, since both PEP 649 and PEP 749 semantics will work for us here. I'd personally prefer that we use the newer syntax rather than stick with the older Optional/Union-based syntax since it's clearer. |
I'm in favor of doing this. |
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 automated commits should be added to .git-blame-ignore-revs
but otherwise LTGM. I skimmed through the changes file by file and didn't see anything obviously wrong.
This enables use of Python 3.10+ type annotation syntax, reducing the churn that would be caused when moving to Python 3.10.
3cae184
to
65fe65b
Compare
Alrighty, I've rebased this on
I agree, but I'd prefer to do it in a follow-up PR once we know the hashes are "final". :) |
Closes #10725
This enables and enforces (via linting) Python 3.10+ style type annotations across the codebase.
x-ref #13182, which has the wonderful statement "currently pip coding style is to simply avoid PEP 585- and PEP 604-style type annotations" that would no longer be true after this PR.
x-ref #13236
This PR is going to be easiest to review commit-by-commit, but practically speaking, the first and last commit are almost entirely handled by Ruff itself. astral-sh/ruff#17763 was a slightly annoying aspect of enabling FA100 but that should be less of a concern moving forward since a PR introducing new annotations can also introduce the relevant import.