Ignore position if imprecise arguments are matched by name #16471
Conversation
This comment has been minimized.
This comment has been minimized.
| def test(x: int) -> int: ... | ||
| apply(apply, test, x=42) # OK | ||
| apply(apply, test, 42) # Also OK (but requires some special casing) | ||
| apply(apply, test, "bad") # E: Argument 1 to "apply" has incompatible type "Callable[[Callable[P, T], **P], None]"; expected "Callable[[Callable[[int], int], str], None]" |
There was a problem hiding this comment.
Off-topic for this PR, and I'm guessing this might be tricky to implement, but it would be really nice if mypy could complain about argument 2 in this error message rather than argument 1. In real-world code, the "mistake" in calls like this is generally passing the wrong kinds of object as arguments after the function, rather than passing in the wrong function altogether
There was a problem hiding this comment.
Yes, this is really hard.
| backend="asyncio", | ||
| )) | ||
| submit( | ||
| run, # E: Argument 1 to "submit" has incompatible type "Callable[[Callable[[], T_Retval], VarArg(object), DefaultNamedArg(str, 'backend')], T_Retval]"; expected "Callable[[Callable[[], Result], int], Result]" |
There was a problem hiding this comment.
Also off-topic for this PR, but I find it unfortunate that we use these mypy_extensions types in error messages, given that we document them as deprecated :(
There was a problem hiding this comment.
I actually was thinking about the same thing myself as I was looking at this test. This is not hard, but tedious. It would be great if someone will do this.
|
The primer output looks really good. 2/4 of the primer hits are places where people are passing overloaded functions (such as |
Co-authored-by: Alex Waygood <[email protected]>
In principle yes, but this is not really related to this PR. |
I actually didn't try, but I think there may be false negatives where two actual arguments are mapped to the same formal argument in the callable if they have the same type, and one is positional, and the second is named. |
Okay, I think I see (though I'm still struggling to think up an actual example of code where this would cause an issue). In any case, that seems like very much an edge case, so I agree that it's better to prioritise avoiding false positive here |
This comment has been minimized.
This comment has been minimized.
|
Diff from mypy_primer, showing the effect of this PR on open source code: pytest (https://github.com/pytest-dev/pytest)
+ src/_pytest/_py/path.py:763: error: Unused "type: ignore" comment [unused-ignore]
+ src/_pytest/_py/path.py:1280: error: Unused "type: ignore" comment [unused-ignore]
poetry (https://github.com/python-poetry/poetry)
+ tests/utils/test_cache.py:345: error: Unused "type: ignore" comment [unused-ignore]
anyio (https://github.com/agronholm/anyio)
+ src/anyio/from_thread.py:395: error: Unused "type: ignore" comment [unused-ignore]
discord.py (https://github.com/Rapptz/discord.py): typechecking got 1.07x faster (171.9s -> 160.2s)
(Performance measurements are based on a single noisy sample)
|
Fixes #16405 Fixes #16412 Imprecise argument kinds inference was added a while ago to support various edge cases with `ParamSpec`. This feature required mapping actual kinds to formal kinds, which is in general undecidable. At that time we decided to not add much special-casing, and wait for some real use-cases. So far there are two relevant issues, and it looks like both of them can be fixed with simple special-casing: ignore argument positions in subtyping if arguments can be matched by name. This adds minor unsafety, and generally doesn't look bad, so I think we should go ahead with it. --------- Co-authored-by: Alex Waygood <[email protected]>
Fixes #16405
Fixes #16412
Imprecise argument kinds inference was added a while ago to support various edge cases with
ParamSpec. This feature required mapping actual kinds to formal kinds, which is in general undecidable. At that time we decided to not add much special-casing, and wait for some real use-cases. So far there are two relevant issues, and it looks like both of them can be fixed with simple special-casing: ignore argument positions in subtyping if arguments can be matched by name. This adds minor unsafety, and generally doesn't look bad, so I think we should go ahead with it.