Skip to content
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

Invalid type inference for zip in a generic class #12588

Closed
JukkaL opened this issue Apr 14, 2022 · 5 comments · Fixed by #12590
Closed

Invalid type inference for zip in a generic class #12588

JukkaL opened this issue Apr 14, 2022 · 5 comments · Fixed by #12590
Labels
bug mypy got something wrong topic-self-types Types for self

Comments

@JukkaL
Copy link
Collaborator

JukkaL commented Apr 14, 2022

The inferred type is wrong here, after the recent typeshed sync:

from typing import TypeVar, Generic, List

T = TypeVar('T')

class C(Generic[T]):
    a: List[T]
    b: List[str]

    def f(self) -> None:
        for x, y in zip(self.a, self.b):
            reveal_type((x, y))  # Tuple[Tuple[T`1, builtins.str], builtins.str]

The correct type would be Tuple[T, builtins.str].

Here's a self-contained example that doesn't depend on typeshed:

from typing import TypeVar, Generic, Iterator, List, Tuple

_T_co = TypeVar("_T_co", covariant=True)
_T1 = TypeVar("_T1")
_T2 = TypeVar("_T2")
S = TypeVar("S")

class Z(Iterator[_T_co]):
    def __new__(cls,
                __iter1: List[_T1],
                __iter2: List[_T2]) -> Z[Tuple[_T1, _T2]]: ...
    def __iter__(self: S) -> S: ...
    def __next__(self) -> _T_co: ...

T = TypeVar('T')

class C(Generic[T]):
    a: List[T]
    b: List[str]

    def f(self) -> None:
        for x, y in Z(self.a, self.b):
            reveal_type((x, y))  # Tuple[Tuple[T`1, builtins.str], builtins.str]

The behavior is correct if we avoid using a self type with __iter__:

    def __iter__(self) -> Z[_T_co]: ...

This happens with some frequency in an internal codebase so this seems important to fix.

@JukkaL JukkaL added bug mypy got something wrong topic-self-types Types for self labels Apr 14, 2022
@JukkaL
Copy link
Collaborator Author

JukkaL commented Apr 14, 2022

I think that this is caused by confusion between type variables of different classes (_T_co vs T) that share the same type variable id. A potential fix for this could be to give each class type variable a unique ID. This way there shouldn't be any confusion.

This would probably also imply updating the type to string conversion logic to not include type variable ids (currently shown as T`<id>). I don't think that the best option is to just drop the type variable ids from messages, since there can be multiple logically separate type variables with the same name. One option would be to use T, T', T'' instead of T`1, T`2, etc. (where the first type variable with short name X doesn't get a prime, the second one gets a single prime, etc.).

@erictraut
Copy link

FWIW, the approach I adopted in pyright for distinguishing between type variables in error messages is to use the form T@<scope> where "scope" is the name of the class, function or type alias that the TypeVar is scoped to. In the above example, the revealed type is tuple[T@C, str]. Users seem to like this because it's clear what it means, and it provides them useful information about where the TypeVar originated. (I will admit that I never understood what T'<id> meant in mypy error messages until just now when I read your explanation, and I'm still not sure how to make use of the id when interpreting an error message.)

@JukkaL
Copy link
Collaborator Author

JukkaL commented Apr 14, 2022

The numeric suffix has only really been useful for debugging type inference bugs by mypy developers. The T@C syntax could be a good way to represent type variables. I will experiment with it.

@hauntsaninja
Copy link
Collaborator

#11657 is probably related

@JelleZijlstra
Copy link
Member

#12610 looks similar too, though it involves enumerate() instead of zip().

JukkaL added a commit that referenced this issue Apr 19, 2022
This avoids confusion between type variables of two classes, which can
happen at least in some edge cases. Type variables are only the same if
both the numeric id and namespace match (plus meta level).

Fixes #12588 (though the textual presentation of type variables doesn't
take the namespace into consideration yet).
JukkaL added a commit that referenced this issue Apr 19, 2022
This avoids confusion between type variables of two classes, which can
happen at least in some edge cases. Type variables are only the same if
both the numeric id and namespace match (plus meta level).

Fixes #12588 (though the textual presentation of type variables doesn't
take the namespace into consideration yet).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong topic-self-types Types for self
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants