Lenient handling of trivial Callable suffixes #15913
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment has been minimized.
This comment has been minimized.
|
Few new errors in Overall I am not impressed TBH: it looks like there are improvements only in 3 out of 140 projects in Btw maybe we need to rethink inference against unions? (maybe even try some form of backtracking?) I may play with this, but finishing |
|
Sure, hauntsaninja/mypy_primer#96 Didn't take a close look at PR itself, but this kind of thing is reasonable and is potentially consistent with the kind of decision we made in #5876 |
This comment has been minimized.
This comment has been minimized.
|
Hm, the |
|
Maybe we could only special case |
It would be even less acceptable. Having two subtly different ways to express essentially the same thing is always a bad idea. |
|
Although there is not much support I see in comments here, there is actually one case where this will be very useful: mypy often does a check like So here is a plan I propose:
|
ilevkivskyi
changed the title
|
OK, I updated the PR title and description to represent the first step of the two-step plan above. |
This comment has been minimized.
This comment has been minimized.
|
Oh well, we got the "bad" thing (in Hm, this actually makes me think that the cases that I thought may cause problem if we do lenient subtyping unconditionally actually involve @JukkaL @hauntsaninja @JelleZijlstra do you have any opinions here? |
This comment has been minimized.
This comment has been minimized.
|
@JukkaL Ping on this. You promised to read and add your opinion :-) |
hauntsaninja
reviewed
Sep 7, 2023
JukkaL
reviewed
Sep 7, 2023
There was a problem hiding this comment.
I like the general idea, and I think the mypy_primer regressions are not too bad, since this enables several new use cases that seem consistent with my expectations of what should work. However, I'm not sure if this is the best way to implement this. Let's think about ways of fixing the issues while introducing less extra complexity to callable types. I left one idea as a comment. And sorry for the slow response.
mypy/types.py
Outdated
| @@ -1778,6 +1778,7 @@ class CallableType(FunctionLike): | |||
| # (this is used for error messages) | |||
| "imprecise_arg_kinds", | |||
| "unpack_kwargs", # Was an Unpack[...] with **kwargs used to define this callable? | |||
| "erased", # Is this callable created as an erased form of a more precise type? | |||
There was a problem hiding this comment.
This feels kind of ad hoc -- and there seems to overlap with is_ellipsis_args. Is there a way to merge these two? For example, allow is_ellipsis_args to be used with an argument prefix. Then erasing a ParamSpec could produce a type with is_ellipsis_args=True.
There was a problem hiding this comment.
Yeah. This looks quite ad-hoc. The more think about it the more I think we should do this unconditionally. This will make the whole thing much simpler.
ilevkivskyi
changed the title
|
OK, I updated the PR title and description again. I think we should do this unconditionally. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Closed
|
Oh well, this exposes a (yet another) bug in variadic types support. Fortunately my next PR will fix the bug, so this PR will need to wait a bit. |
ilevkivskyi
added a commit
that referenced
this pull request
Sep 13, 2023
The second part of support for user defined variadic types comes as a single PR, it was hard to split into smaller parts. This part covers subtyping and inference (and relies on the first part: type analysis, normalization, and expansion, concluded by #15991). Note btw that the third (and last) part that covers actually using all the stuff in `checkexpr.py` will likely come as several smaller PRs. Some comments on this PR: * First good news: it looks like instances subtyping/inference can be handled in a really simple way, we just need to find correct type arguments mapping for each type variable, and perform procedures argument by argument (note this heavily relies on the normalization). Also callable subtyping inference for variadic items effectively defers to corresponding tuple types. This way all code paths will ultimately go through variadic tuple subtyping/inference (there is still a bunch of boilerplate to do the mapping, but it is quite simple). * Second some bad news: a lot of edge cases involving `*tuple[X, ...]` were missing everywhere (even couple cases in the code I touched before). I added all that were either simple or important. We can handle more if users will ask, since it is quite tricky. * Note that I handle variadic tuples essentially as infinite unions, the core of the logic for this (and for most of this PR FWIW) is in `variadic_tuple_subtype()`. * Previously `Foo[*tuple[int, ...]]` was considered a subtype of `Foo[int, int]`. I think this is wrong. I didn't find where this is required in the PEP (see one case below however), and mypy currently considers `tuple[int, ...]` not a subtype of `tuple[int, int]` (vice versa are subtypes), and similarly `(*args: int)` vs `(x: int, y: int)` for callables. Because of the logic I described in the first comment, the same logic now uniformly applies to instances as well. * Note however the PEP requires special casing of `Foo[*tuple[Any, ...]]` (equivalent to bare `Foo`), and I agree we should do this. I added a minimal special case for this. Note we also do this for callables as well (`*args: Any` is very different from `*args: object`). And I think we should special case `tuple[Any, ...] <: tuple[int, int]` as well. In the future we can even extend the special casing to `tuple[int, *tuple[Any, ...], int]` in the spirit of #15913 * In this PR I specifically only handle the PEP required item from above for instances. For plain tuples I left a TODO, @hauntsaninja may implement it since it is needed for other unrelated PR. * I make the default upper bound for `TypeVarTupleType` to be `tuple[object, ...]`. I think it can never be `object` (and this simplifies some subtyping corner cases). * TBH I didn't look into callables subtyping/inference very deeply (unlike instances and tuples), if needed we can improve their handling later. * Note I remove some failing unit tests because they test non-nomralized forms that should never appear now. We should probably add some more unit tests, but TBH I am quite tired now.
|
Diff from mypy_primer, showing the effect of this PR on open source code: pandas (https://github.com/pandas-dev/pandas)
+ pandas/core/arrays/timedeltas.py:293: error: Unused "type: ignore" comment [unused-ignore]
+ pandas/core/arrays/datetimes.py:381: error: Unused "type: ignore" comment [unused-ignore]
+ pandas/core/arrays/categorical.py:2861: error: Unused "type: ignore" comment [unused-ignore]
+ pandas/core/indexes/accessors.py:90: error: Unused "type: ignore" comment [unused-ignore]
+ pandas/core/indexes/accessors.py:178: error: Unused "type: ignore" comment [unused-ignore]
steam.py (https://github.com/Gobot1234/steam.py)
- steam/http.py:175: error: Value of type variable "F_wait" of function cannot be "Callable[[HTTPClient], Coroutine[Any, Any, None]]" [type-var]
- steam/http.py:190: error: Value of type variable "F_wait" of function cannot be "Callable[[HTTPClient], Coroutine[Any, Any, str | None]]" [type-var]
- steam/gateway.py:630: error: Value of type variable "F_wait" of function cannot be "Callable[[SteamWebSocket], Coroutine[Any, Any, str]]" [type-var]
- steam/state.py:1762: error: Value of type variable "F_no_wait" of "call_once" cannot be "Callable[[ConnectionState], Coroutine[Any, Any, None]]" [type-var]
- steam/ext/tf2/state.py:152: error: Value of type variable "F_no_wait" of "call_once" cannot be "Callable[[GCState], Coroutine[Any, Any, None]]" [type-var]
graphql-core (https://github.com/graphql-python/graphql-core): typechecking got 1.05x slower (292.0s -> 306.9s)
(Performance measurements are based on a single noisy sample)
Tanjun (https://github.com/FasterSpeeding/Tanjun)
- tanjun/dependencies/limiters.py:1302: error: Argument 1 to "add_pre_execution" of "Hooks" has incompatible type "CooldownPreExecution"; expected "Callable[[Context, VarArg(Any), KwArg(Any)], Optional[Coroutine[Any, Any, None]]]" [arg-type]
- tanjun/dependencies/limiters.py:1302: note: "CooldownPreExecution.__call__" has type "Callable[[Context, Arg(AbstractCooldownManager, 'cooldowns'), DefaultNamedArg(Optional[AbstractLocaliser], 'localiser'), NamedArg(Optional[AbstractOwners], 'owner_check')], Coroutine[Any, Any, None]]"
- tanjun/dependencies/limiters.py:1302: error: Argument 1 to "add_pre_execution" of "Hooks" has incompatible type "CooldownPreExecution"; expected "Callable[[Any, VarArg(Any), KwArg(Any)], Optional[Coroutine[Any, Any, None]]]" [arg-type]
- tanjun/dependencies/limiters.py:1302: error: Argument 1 to "add_post_execution" of "Hooks" has incompatible type "CooldownPostExecution"; expected "Callable[[Context, VarArg(Any), KwArg(Any)], Optional[Coroutine[Any, Any, None]]]" [arg-type]
- tanjun/dependencies/limiters.py:1302: note: "CooldownPostExecution.__call__" has type "Callable[[Context, NamedArg(AbstractCooldownManager, 'cooldowns')], Coroutine[Any, Any, None]]"
- tanjun/dependencies/limiters.py:1302: error: Argument 1 to "add_post_execution" of "Hooks" has incompatible type "CooldownPostExecution"; expected "Callable[[Any, VarArg(Any), KwArg(Any)], Optional[Coroutine[Any, Any, None]]]" [arg-type]
- tanjun/dependencies/limiters.py:1796: error: Argument 1 to "add_pre_execution" of "Hooks" has incompatible type "ConcurrencyPreExecution"; expected "Callable[[Context, VarArg(Any), KwArg(Any)], Optional[Coroutine[Any, Any, None]]]" [arg-type]
- tanjun/dependencies/limiters.py:1796: note: "ConcurrencyPreExecution.__call__" has type "Callable[[Context, Arg(AbstractConcurrencyLimiter, 'limiter'), DefaultNamedArg(Optional[AbstractLocaliser], 'localiser')], Coroutine[Any, Any, None]]"
- tanjun/dependencies/limiters.py:1796: error: Argument 1 to "add_pre_execution" of "Hooks" has incompatible type "ConcurrencyPreExecution"; expected "Callable[[Any, VarArg(Any), KwArg(Any)], Optional[Coroutine[Any, Any, None]]]" [arg-type]
- tanjun/dependencies/limiters.py:1796: error: Argument 1 to "add_post_execution" of "Hooks" has incompatible type "ConcurrencyPostExecution"; expected "Callable[[Context, VarArg(Any), KwArg(Any)], Optional[Coroutine[Any, Any, None]]]" [arg-type]
- tanjun/dependencies/limiters.py:1796: note: "ConcurrencyPostExecution.__call__" has type "Callable[[Context, Arg(AbstractConcurrencyLimiter, 'limiter')], Coroutine[Any, Any, None]]"
- tanjun/dependencies/limiters.py:1796: error: Argument 1 to "add_post_execution" of "Hooks" has incompatible type "ConcurrencyPostExecution"; expected "Callable[[Any, VarArg(Any), KwArg(Any)], Optional[Coroutine[Any, Any, None]]]" [arg-type]
- tanjun/checks.py:624: error: Argument 2 to "_optional_kwargs" has incompatible type "DmCheck"; expected "Callable[[Context, VarArg(Any), KwArg(Any)], Union[Coroutine[Any, Any, bool], bool]]" [arg-type]
- tanjun/checks.py:624: note: "DmCheck.__call__" has type "Callable[[Context, DefaultNamedArg(Optional[AbstractLocaliser], 'localiser')], bool]"
- tanjun/checks.py:690: error: Argument 2 to "_optional_kwargs" has incompatible type "GuildCheck"; expected "Callable[[Context, VarArg(Any), KwArg(Any)], Union[Coroutine[Any, Any, bool], bool]]" [arg-type]
- tanjun/checks.py:690: note: "GuildCheck.__call__" has type "Callable[[Context, DefaultNamedArg(Optional[AbstractLocaliser], 'localiser')], bool]"
- tanjun/checks.py:752: error: Argument 2 to "_optional_kwargs" has incompatible type "NsfwCheck"; expected "Callable[[Context, VarArg(Any), KwArg(Any)], Union[Coroutine[Any, Any, bool], bool]]" [arg-type]
- tanjun/checks.py:752: note: "NsfwCheck.__call__" has type "Callable[[Context, DefaultNamedArg(Optional[AbstractLocaliser], 'localiser')], Coroutine[Any, Any, bool]]"
- tanjun/checks.py:814: error: Argument 2 to "_optional_kwargs" has incompatible type "SfwCheck"; expected "Callable[[Context, VarArg(Any), KwArg(Any)], Union[Coroutine[Any, Any, bool], bool]]" [arg-type]
- tanjun/checks.py:814: note: "SfwCheck.__call__" has type "Callable[[Context, DefaultNamedArg(Optional[AbstractLocaliser], 'localiser')], Coroutine[Any, Any, bool]]"
- tanjun/checks.py:876: error: Argument 2 to "_optional_kwargs" has incompatible type "OwnerCheck"; expected "Callable[[Context, VarArg(Any), KwArg(Any)], Union[Coroutine[Any, Any, bool], bool]]" [arg-type]
- tanjun/checks.py:876: note: "OwnerCheck.__call__" has type "Callable[[Context, Arg(AbstractOwners, 'dependency'), DefaultNamedArg(Optional[AbstractLocaliser], 'localiser')], Coroutine[Any, Any, bool]]"
- tanjun/checks.py:932: error: Argument 2 to "_add_to_command" has incompatible type "AuthorPermissionCheck"; expected "Callable[[Context, VarArg(Any), KwArg(Any)], Union[Coroutine[Any, Any, bool], bool]]" [arg-type]
- tanjun/checks.py:932: note: "AuthorPermissionCheck.__call__" has type "Callable[[Context, DefaultNamedArg(Optional[AbstractLocaliser], 'localiser')], Coroutine[Any, Any, bool]]"
- tanjun/checks.py:989: error: Argument 2 to "_add_to_command" has incompatible type "OwnPermissionCheck"; expected "Callable[[Context, VarArg(Any), KwArg(Any)], Union[Coroutine[Any, Any, bool], bool]]" [arg-type]
- tanjun/checks.py:989: note: "OwnPermissionCheck.__call__" has type "Callable[[Context, DefaultNamedArg(Optional[AbstractLocaliser], 'localiser'), DefaultNamedArg(OwnUser, 'my_user'), DefaultNamedArg(Optional[GuildBoundCache[Union[Snowflake, int], Member]], 'member_cache')], Coroutine[Any, Any, bool]]"
- tanjun/checks.py:1150: error: Argument 2 to "_add_to_command" has incompatible type "Callable[[Context], Coroutine[Any, Any, bool]]"; expected "Callable[[Context, VarArg(Any), KwArg(Any)], Union[Coroutine[Any, Any, bool], bool]]" [arg-type]
- tanjun/checks.py:1339: error: Argument 2 to "_add_to_command" has incompatible type "_AnyCallback[Any]"; expected "Callable[[Context, VarArg(Any), KwArg(Any)], Union[Coroutine[Any, Any, bool], bool]]" [arg-type]
- tanjun/checks.py:1339: note: "_AnyCallback[Any].__call__" has type "Callable[[Any, DefaultNamedArg(Optional[AbstractLocaliser], 'localiser')], Coroutine[Any, Any, bool]]"
- tanjun/clients.py:758: error: Argument 1 to "set_on_parser_error" of "Hooks" has incompatible type "Callable[[Context, ParserError], Coroutine[Any, Any, None]]"; expected "Optional[Callable[[Context, ParserError, VarArg(Any), KwArg(Any)], Union[Coroutine[Any, Any, Optional[bool]], bool, None]]]" [arg-type]
- tanjun/clients.py:1902: error: Argument 1 to "add_check" of "Client" has incompatible type "Callable[[Context], bool]"; expected "Callable[[Context, VarArg(Any), KwArg(Any)], Union[Coroutine[Any, Any, bool], bool]]" [arg-type]
- tanjun/clients.py:1906: error: Argument 1 to "remove_check" of "Client" has incompatible type "Callable[[Context], bool]"; expected "Callable[[Context, VarArg(Any), KwArg(Any)], Union[Coroutine[Any, Any, bool], bool]]" [arg-type]
- tanjun/annotations.py:1454: error: List item 0 has incompatible type "Callable[[str], Enum]"; expected "Callable[[str, VarArg(Any), KwArg(Any)], Union[Coroutine[Any, Any, Any], Any]]" [list-item]
- tanjun/annotations.py:2465: error: Incompatible types in assignment (expression has type "tuple[Callable[[str], Snowflake]]", variable has type "Sequence[Callable[[str, VarArg(Any), KwArg(Any)], Union[Coroutine[Any, Any, Any], Any]]]") [assignment]
- tanjun/annotations.py:2471: error: Incompatible types in assignment (expression has type "tuple[ToChannel]", variable has type "Sequence[Callable[[str, VarArg(Any), KwArg(Any)], Union[Coroutine[Any, Any, Any], Any]]]") [assignment]
- tanjun/annotations.py:2550: error: Argument "converters" to "add_float_option" of "SlashCommand" has incompatible type "Union[Callable[[float], Any], tuple[()]]"; expected "Union[Sequence[Callable[[float, VarArg(Any), KwArg(Any)], Union[Coroutine[Any, Any, Any], Any]]], Callable[[float, VarArg(Any), KwArg(Any)], Union[Coroutine[Any, Any, Any], Any]]]" [arg-type]
- tanjun/annotations.py:2560: error: Argument "converters" to "add_int_option" of "SlashCommand" has incompatible type "Union[Callable[[int], Any], tuple[()]]"; expected "Union[Sequence[Callable[[int, VarArg(Any), KwArg(Any)], Union[Coroutine[Any, Any, Any], Any]]], Callable[[int, VarArg(Any), KwArg(Any)], Union[Coroutine[Any, Any, Any], Any]]]" [arg-type]
discord.py (https://github.com/Rapptz/discord.py)
+ discord/ext/commands/hybrid.py:826: error: Need type annotation for "result" [var-annotated]
+ discord/ext/commands/hybrid.py:826: error: Argument 1 has incompatible type "Callable[[CogT, ContextT, **P2], Coroutine[Any, Any, U]] | Callable[[ContextT, **P2], Coroutine[Any, Any, U]]"; expected "Callable[[Never, Never, VarArg(Never), KwArg(Never)], Coroutine[Any, Any, Never]] | Callable[[Never, VarArg(Never), KwArg(Never)], Coroutine[Any, Any, Never]]" [arg-type]
+ discord/ext/commands/hybrid.py:830: error: Incompatible return value type (got "Callable[[Callable[[CogT, ContextT, **P2], Coroutine[Any, Any, U]] | Callable[[ContextT, **P2], Coroutine[Any, Any, U]]], Any]", expected "Callable[[Callable[[CogT, ContextT, **P2], Coroutine[Any, Any, U]] | Callable[[ContextT, **P2], Coroutine[Any, Any, U]]], HybridCommand[CogT, P2, U]]") [return-value]
+ discord/ext/commands/hybrid.py:850: error: Need type annotation for "result" [var-annotated]
+ discord/ext/commands/hybrid.py:850: error: Argument 1 has incompatible type "Callable[[CogT, ContextT, **P2], Coroutine[Any, Any, U]] | Callable[[ContextT, **P2], Coroutine[Any, Any, U]]"; expected "Callable[[Never, Never, VarArg(Never), KwArg(Never)], Coroutine[Any, Any, Never]] | Callable[[Never, VarArg(Never), KwArg(Never)], Coroutine[Any, Any, Never]]" [arg-type]
+ discord/ext/commands/hybrid.py:854: error: Incompatible return value type (got "Callable[[Callable[[CogT, ContextT, **P2], Coroutine[Any, Any, U]] | Callable[[ContextT, **P2], Coroutine[Any, Any, U]]], Any]", expected "Callable[[Callable[[CogT, ContextT, **P2], Coroutine[Any, Any, U]] | Callable[[ContextT, **P2], Coroutine[Any, Any, U]]], HybridGroup[CogT, P2, U]]") [return-value]
- discord/ext/commands/help.py:228: error: Argument 1 to "__init__" of "Command" has incompatible type "Callable[[Context[BotT], DefaultNamedArg(str | None, 'command')], Coroutine[Any, Any, None]]"; expected "Callable[[Any, Context[Any], VarArg(Any), KwArg(Any)], Coroutine[Any, Any, Any]] | Callable[[Context[Any], VarArg(Any), KwArg(Any)], Coroutine[Any, Any, Any]]" [arg-type]
- discord/ext/commands/help.py:236: error: Incompatible types in assignment (expression has type "Callable[[Context[BotT], DefaultNamedArg(str | None, 'command')], Coroutine[Any, Any, None]]", variable has type "Callable[[Any, Context[Any], VarArg(Any), KwArg(Any)], Coroutine[Any, Any, Any]] | Callable[[Context[Any], VarArg(Any), KwArg(Any)], Coroutine[Any, Any, Any]]") [assignment]
+ discord/ext/commands/bot.py:290: error: Need type annotation for "result" [var-annotated]
+ discord/ext/commands/bot.py:290: error: Argument 1 has incompatible type "Callable[[Any, ContextT, **P], Coroutine[Any, Any, T]] | Callable[[ContextT, **P], Coroutine[Any, Any, T]]"; expected "Callable[[Never, Never, VarArg(Never), KwArg(Never)], Coroutine[Any, Any, Never]] | Callable[[Never, VarArg(Never), KwArg(Never)], Coroutine[Any, Any, Never]]" [arg-type]
+ discord/ext/commands/bot.py:294: error: Incompatible return value type (got "Callable[[Callable[[Any, ContextT, **P], Coroutine[Any, Any, T]] | Callable[[ContextT, **P], Coroutine[Any, Any, T]]], Any]", expected "Callable[[Callable[[Any, ContextT, **P], Coroutine[Any, Any, T]] | Callable[[ContextT, **P], Coroutine[Any, Any, T]]], HybridCommand[Any, P, T]]") [return-value]
+ discord/ext/commands/bot.py:314: error: Need type annotation for "result" [var-annotated]
+ discord/ext/commands/bot.py:314: error: Argument 1 has incompatible type "Callable[[Any, ContextT, **P], Coroutine[Any, Any, T]] | Callable[[ContextT, **P], Coroutine[Any, Any, T]]"; expected "Callable[[Never, Never, VarArg(Never), KwArg(Never)], Coroutine[Any, Any, Never]] | Callable[[Never, VarArg(Never), KwArg(Never)], Coroutine[Any, Any, Never]]" [arg-type]
+ discord/ext/commands/bot.py:318: error: Incompatible return value type (got "Callable[[Callable[[Any, ContextT, **P], Coroutine[Any, Any, T]] | Callable[[ContextT, **P], Coroutine[Any, Any, T]]], Any]", expected "Callable[[Callable[[Any, ContextT, **P], Coroutine[Any, Any, T]] | Callable[[ContextT, **P], Coroutine[Any, Any, T]]], HybridGroup[Any, P, T]]") [return-value]
|
|
Now that all is green, I am going to merge this unless there are any objections. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #15734
Fixes #15188
Fixes #14321
Fixes #13107 (plain Callable was already working, this fixes the protocol example)
Fixes #16058
It looks like treating trivial suffixes (especially for erased callables) as "whatever works" is a right thing, because it reflects the whole idea of why we normally check subtyping with respect to an e.g. erased type. As you can see this fixes a bunch of issues. Note it was necessary to make couple more tweaks to make everything work smoothly:
checker.pyto match other places.Callableas aself/clsannotation (actually I am not sure we need to keep this check at all, since we now have good inference for self-types, and we check they are safe either at definition site or at call site).