-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Upgrade to mypy 1.12.1 #13022
Upgrade to mypy 1.12.1 #13022
Conversation
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.
Could you comment "pre-commit.ci autofix" @hauntsaninja? Thanks :)
Thanks for the review! re Protocol: I thought we needed a different fix, but then noticed I could get away with something much simpler |
@@ -80,7 +80,7 @@ def test_make_metadata_file_custom_value_overrides() -> None: | |||
|
|||
def test_make_metadata_file_custom_contents() -> None: | |||
value = b"hello" | |||
f = default_make_metadata(value=value) | |||
f = default_make_metadata(value=value) # type: ignore[arg-type] |
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.
Do you know the reason of this one? Is that some regression in mypy?
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.
Prior to mypy 1.11, mypy did not understand functools.partial at all, so calls here were effectively going unchecked. mypy now struggles a little to solve the value constrained type var once it's been partially solved in the functools.partial call.
Looking at it a little closer, the type variable in make_wheel_metadata_file
is actually unnecessary, so I'll remove it to make one of the two of these go away
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.
LGTM. Could you remove the last 2 commits (1.12.1 + revert)?
Thanks for the review! I squashed the commits. I also landed 1.12.1 (I'd reverted because pre-commit didn't support it yet, but it does now) |
Thank you @hauntsaninja ! |
No description provided.