Skip to content

Variable incorrectly typed as None, misleading error message #1691

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

Closed
dmoisset opened this issue Jun 10, 2016 · 7 comments · Fixed by #2465
Closed

Variable incorrectly typed as None, misleading error message #1691

dmoisset opened this issue Jun 10, 2016 · 7 comments · Fixed by #2465
Labels
bug mypy got something wrong

Comments

@dmoisset
Copy link
Contributor

dmoisset commented Jun 10, 2016

While debugging a more complicated issue, I reduced it to this:

from typing import Tuple, Union

Pair = Tuple[int, int]
Variant = Union[int, Pair]


def tuplify(v: Variant) -> None:
    if not isinstance(v, tuple):
        v = (v, v)
    print(v[0], v[1])

mypy running on this example detects an error on the print line saying:

foo.py:10: error: Value of type None is not indexable

For some reason it says that the value of v has type None. I understand why the checker may not be able to follow the negative isinstance check, but I expected the error to say something like Value of type Union[...] is not indexable.

@ddfisher ddfisher added the bug mypy got something wrong label Jun 10, 2016
@ddfisher
Copy link
Collaborator

This looks like a bug to me. If you change the check from not isinstance(v, tuple) to isinstance(v, int) (which in this case should be the same) then mypy passes without errors. The logic around these isinstance checks is pretty complicated -- we probably have a slightly messed up case.

@rwbarton
Copy link
Contributor

A minimal reproducer:

from typing import Tuple, Union

Pair = Tuple[int, int]
Variant = Union[int, Pair]

def tuplify(v: Variant) -> None:
    if isinstance(v, tuple):
        reveal_type(v)  # E: Revealed type is 'builtins.None'

One way to fix this would be to make meet_types of Tuple[int, int] and tuple[Any] return Tuple[int, int] (as it should because Tuple[int, int] is a subtype of tuple[Any]). I think the whole approach of using meet in isinstance checks is wrong though; instead we should have functions of types

def restrict_subtype_to(e: Type, t: TypeInfo) -> Type: ...
def restrict_subtype_away(e: Type, t: TypeInfo) -> Type: ...

since the second argument of isinstance has to be a type object (or tuple of type objects) and never anything complicated like a generic class with specific type arguments, or Any, etc.

@dmoisset
Copy link
Contributor Author

I think it shouldn't be correct that «Tuple[int, int] is a subtype of tuple[Any]» (Any is actually something that should work as being subtype of everything, given that any operation on it is invalid). I don't know how mypy implements it, my statement is from an understanding of type systems outside of mypy.

I think the problem here should be that isinstance(v, tuple) should join the type with Tuple[object...] instead of joining with Tuple[Any...], because checking isinstance(v, tuple) doesn't give you any information at all about the elements of the tuple and object is the type that means "I have a value with no information about it".

@ddfisher
Copy link
Collaborator

Any doesn't participate in normal subtyping relations, because it's meant to be used as an escape hatch. It's actually both at the bottom and the top of the subtyping hierarchy (loosely speaking).

PEP 484 implies that list is supposed to be equivalent to List[Any] in types. (From a quick skim, I'm not sure if it says this outright.) I don't think that necessarily implies that we have to treat it that way in isinstance checks, though. I think @dmoisset's suggestion is a good one -- we should consider treating containers as object filled rather than Any filled in isinstance checks.

@rwbarton
Copy link
Contributor

That makes sense for covariant parameters, but might be a little weird for invariant and especially contravariant parameters. At any rate, if v has type Union[int, List[int]] and an isinstance(v, list) check is True, then the refined type of v must be List[int], which is not the meet of List[int] and List[object] (since it's not a subtype of List[object]).

On the other hand, we can ask the question

def f(x: object) -> None:
    if isinstance(x, list):
        ... # what is the type of x here?

If the type is List[object], then f could append any sort of object to a provided argument of type List[int], which is unsound. Of course, the current choice List[Any] is even more unsound. Perhaps the theoretically correct thing to do would be to treat the type as exists T. List[T]; introduce a new type variable T (that is treated like a generic function type parameter) inside the block and give x type List[T]. This doesn't actually sound that hard, but does seem rather low priority...

@ddfisher
Copy link
Collaborator

For future reference, here's a simpler repro of the problem:

from typing import Tuple, Union

x = (0, 0)  # type: Union[Tuple[int,int], int]
if isinstance(x, tuple):
    reveal_type(x)  # E: Revealed type is 'builtins.None'
else:
    reveal_type(x)  # E: Revealed type is 'builtins.int'

@euresti
Copy link
Contributor

euresti commented Jul 15, 2016

I also find it leaves the variable in a weird state:

def bad(blah):
    # type: (Union[Tuple[int, int], int]) -> None
    reveal_type(blah)  # blah is Union[Tuple[int, int], int]
    if isinstance(blah, tuple):
        reveal_type(blah)   # blah is now None?
    reveal_type(blah)  # blah is now int?

Note: if you reverse the instance check to check for the other side it works fine:

def good(blah):
    # type: (Union[Tuple[int, int], int]) -> None
    reveal_type(blah)  # blah is Union[Tuple[int, int], int]
    if isinstance(blah, int):
        reveal_type(blah)   # blah is now int
    else:
        reveal_type(blah)   # blah is Tuple[int, int]
    reveal_type(blah)  # blah is Union[Tuple[int, int], int]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants