Make sure to keep type variables even if unused.#15248
Make sure to keep type variables even if unused.#15248A5rocks wants to merge 1 commit intopython:masterfrom
Conversation
This is useful if (via ParamSpec) you get `[T] () -> (T) -> T`. Otherwise this would become `(<nothing>) -> <nothing>` which is just wrong! This fixes python#12595.
This comment has been minimized.
This comment has been minimized.
|
Did not realize renaming branches via GitHub's interface did that. |
|
Diff from mypy_primer, showing the effect of this PR on open source code: dacite (https://github.com/konradhalas/dacite) got 3.40x faster (7.5s -> 2.2s)
prefect (https://github.com/PrefectHQ/prefect)
- src/prefect/client/base.py:36: error: Argument 1 to "asynccontextmanager" has incompatible type "Callable[[Any], AbstractContextManager[None]]"; expected "Callable[[Any], AsyncIterator[<nothing>]]" [arg-type]
+ src/prefect/client/base.py:36: error: Argument 1 to "asynccontextmanager" has incompatible type "Callable[[Any], AbstractContextManager[None]]"; expected "Callable[[Any], AsyncIterator[_T_co]]" [arg-type]
- src/prefect/context.py:380: error: Argument 1 to "contextmanager" has incompatible type "Callable[[VarArg(str)], set[str]]"; expected "Callable[[VarArg(str)], Iterator[<nothing>]]" [arg-type]
+ src/prefect/context.py:380: error: Argument 1 to "contextmanager" has incompatible type "Callable[[VarArg(str)], set[str]]"; expected "Callable[[VarArg(str)], Iterator[_T_co]]" [arg-type]
ibis (https://github.com/ibis-project/ibis)
- ibis/backends/tests/test_client.py:1208: error: Argument 1 to "contextmanager" has incompatible type "Callable[[BaseBackend], str]"; expected "Callable[[BaseBackend], Iterator[<nothing>]]" [arg-type]
+ ibis/backends/tests/test_client.py:1208: error: Argument 1 to "contextmanager" has incompatible type "Callable[[BaseBackend], str]"; expected "Callable[[BaseBackend], Iterator[_T_co]]" [arg-type]
|
ilevkivskyi
left a comment
ilevkivskyi
left a comment
There was a problem hiding this comment.
Sorry, but I don't think this is a good solution.
| def f(x: Optional[T] = None) -> Callable[..., T]: ... | ||
|
|
||
| x = f() # E: Need type annotation for "x" | ||
| # Question: Is this alright? |
There was a problem hiding this comment.
No, this test specifically tests the error will be shown.
| # def () -> def [T] (T) -> T | ||
| for i, argument in enumerate(inferred_args): | ||
| if isinstance(get_proper_type(argument), UninhabitedType): | ||
| # un-"freshen" the type variable :^) |
There was a problem hiding this comment.
I don't like this. A type def [T] () -> <anything with T> doesn't make sense, and we should figure out why it was inferred/constructed in the first place. This is really just sweeping dust under the carpet.
There was a problem hiding this comment.
I can try to explain why this happens:
Have a function that takes a Callable[Concatenate[T, P], ...] and then returns a Callable[P, Callable[[T], ...]]. This could be some utility function that curries (in reverse), for example.
Now, the problem is that if you use this on def [T] (T, int) -> ... then this will turn into... def [T] (int) -> (T) -> .... At least, as currently implemented. Thus, I see two solutions:
- Fix the new function at call time (what I do here) when we have all this information
- Fix the function at construction time, which I'm not sure how to do: I'm not quite sure how to scan through the parameters to find if type variables are still used. For example, this should still work:
def [T] (T, T) -> ...intodef [T] (T) -> T -> ....
Sorry if this is a confusing explanation!
There was a problem hiding this comment.
Ah just to be clear, the problem with def [T] (int) -> (T) -> ... is we'll infer an UninhabitedType for T, meaning that we can't actually trust the return type anymore.
There was a problem hiding this comment.
Interesting. I am actually working on a (tangentially related) PR that will likely fix this (or at least will allow a more principled solution). I will post a link here when it is ready.
| transform: Callable[Concatenate[T, P], U] | ||
| ) -> Callable[P, Callable[[T], U]]: ... | ||
|
|
||
| u = pop_off(t) |
There was a problem hiding this comment.
TBH I don't think this situation is what is going on in the issue. IIUC you tried to simplify the example, but it seems to me you may have simplified too much.
|
I'll close this while waiting for that other PR. Also, this way I have one less PR to worry over! Just wanted to add one more thing: while mypy's current behavior could been seen as right ("you shouldn't pop off parameters like that"), I believe the pragmatic choice is to support some form of this and the API I want for my library necessitates this! |
3 participants
I'm finally breaking up #14903 into smaller pieces. Sorry about that!
When mypy sees a typevar is unconstrained, it tries to set it to
<nothing>(UninhabitedType). This PR modifies this keep the typevar in the returned callable. This is useful if (via ParamSpec) you get[T] () -> (T) -> T. Otherwise this would become(<nothing>) -> <nothing>which is just bad, I think! I believe this is significantly simpler than actually catching that ParamSpec case (in which you'd have to tell if theTypeVaris used somewhere in the args.), but if it isn't that is probably cleaner.This fixes #12595.