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

Passing enum directly as iterable loses type specificity #3210

Closed
FuegoFro opened this issue Apr 21, 2017 · 15 comments · Fixed by python/typeshed#1755
Closed

Passing enum directly as iterable loses type specificity #3210

FuegoFro opened this issue Apr 21, 2017 · 15 comments · Fixed by python/typeshed#1755
Labels
bug mypy got something wrong priority-1-normal

Comments

@FuegoFro
Copy link
Contributor

It looks like if you pass an Enum class directly to a generic that wants an iterable (eg list, frozenset) it will forget which type of Enum you have. However if you make a no-op iterable out of it first, the enum type is preserved:

from enum import Enum

class Colors(Enum):
    RED = 0
    BLUE = 1

reveal_type(frozenset(c for c in Colors))  # <-- Is `frozenset[Colors*]`
reveal_type(frozenset(Colors))  # <-- Is `frozenset[Enum*]`

The last line here should have type frozenset[Colors*] rather than frozenset[Enum*]

@gvanrossum
Copy link
Member

Confirmed, and this is not entirely limited to Enum. It seems to have to do with an iterable metaclass. Here's a simpler example:

from typing import *

T = TypeVar('T', bound='E')

class EMeta(type, Iterable['E']):
    def __iter__(self: Type[T]) -> Iterator[T]: ...

class E(metaclass=EMeta):
    pass

class C(E):
    pass

reveal_type(list(c for c in C))  # list[C]
reveal_type(list(C))  # list[E]

@elazarg Do you have an idea? (IIRC you added metaclass support?)

@elazarg
Copy link
Contributor

elazarg commented Apr 22, 2017

I think list tries to infer_type_arguments_using_context, and the context is Iterable[E].

There was a brief discussion about this in some other issue/PR; part of the the problem is the absence of syntax for declaring the actual intention; so protocols may help, but I did not yet experiment with the combination of protocols, self types and metaclasses though.

@gvanrossum
Copy link
Member

Why would the context for list(C) be Iterable[E]? (The comments in my example indicate the output, they are not intended as type comments.)

I don't think there's special-casing for list() in mypy, it just sees the class definition for list and picks the __init__ overload that matches the argument count. So then C is evaluated in the context Iterable[X] for type variable X. Then something must be picking the metaclass and filling in the wrong type for X. This would go to the metaclass EnumMeta, whose __iter__ uses a type variable with upper bound Enum, so that's apparently what we get, even though C would be another solution.

@ilevkivskyi This smells familiar -- does it ring a bell?

@elazarg
Copy link
Contributor

elazarg commented Apr 22, 2017

Something is picking the metaclass, looking at the declaration: class EMeta(type, Iterable['E']) and filling it with the declared type E.

The way I understand it, the absence of special casing is the source of the issue here. The type C is not a valid solution unless you treat Iterable structurally (which what comprehensions do).

@gvanrossum
Copy link
Member

Ah, I think you're right and now I understand the issue.

I have no solution alas.

@elazarg
Copy link
Contributor

elazarg commented Apr 22, 2017

It also has nothing to do with metaclasses:

from typing import TypeVar
T = TypeVar('T', bound='A')

class A(Iterable['A']):
    def __iter__(self: T) -> Iterator[T]: ...

class B(A): pass

reveal_type(list(b for b in B()))  # list[B]
reveal_type(list(B()))  # list[A]

@ilevkivskyi
Copy link
Member

Protocols solve this problem if I remove the explicit Iterable bases in all examples (they are not precise enough and my PR picks up nominal first).

In principle my PR plays nicely with self types and metaclasses, mainly I just reuse checkmember.bind_self and callable fallbacks. Sometimes I find bugs/corner cases that could be fixed also in nominal code but I don't touch that code (for example to make the second example work it was necessary to preserve the original callable type instead of just immediately switching to metaclass fallback, def () -> C is more precise than EMeta, this is something that can be also useful for nominal code).

@elazarg
Copy link
Contributor

elazarg commented Apr 23, 2017

Well, I think this kind of things worth separate issues and PRs.

Maybe you can whitelist Iterable and some other "built in" protocols, so they won't be nominal-first?

@elazarg
Copy link
Contributor

elazarg commented Apr 23, 2017

Or maybe an "infer" parameter will do this whitelisting.

@ilevkivskyi
Copy link
Member

@elazarg Yes, we could play with this. The general plan is to make a "smooth" transition. Initially, only mypy protocols PR will be (hopefully) merged, but typeshed classes will stay nominal, then we could gradually replace nominal classes with corresponding protocols, then we could maybe even switch to structural first for some classes.

@gvanrossum
Copy link
Member

@elazarg please go ahead and create the separate issues you see necessary.

@elazarg
Copy link
Contributor

elazarg commented Nov 20, 2017

Seems like it's getting worse with protocols (regression?):

$ cat tmp.py
from enum import Enum

class Colors(Enum):
    RED = 0
    BLUE = 1

reveal_type(frozenset(c for c in Colors))  # line 7
reveal_type(frozenset(Colors))  # line 8
$ mypy tmp.py
tmp.py:7: error: Revealed type is 'builtins.frozenset[Any]'
tmp.py:7: error: Iterable expected
tmp.py:7: error: "Colors" has no attribute "__iter__"
tmp.py:8: error: Revealed type is 'builtins.frozenset[<uninhabited>]'
tmp.py:8: error: Argument 1 to "frozenset" has incompatible type "Colors"; expected Iterable[<uninhabited>]

@ilevkivsky is this the constraints problem you talked about?

@ilevkivskyi
Copy link
Member

@elazarg Strange, I just installed from master and I just get this:

tst23.py:7: error: Revealed type is 'builtins.frozenset[tst23.Colors*]'
tst23.py:8: error: Revealed type is 'builtins.frozenset[enum.Enum*]'

(which is the same as before)

@elazarg
Copy link
Contributor

elazarg commented Nov 20, 2017

It's likely there's a problem on my side (I don't know what it is, but I'm using a new machine) - sorry about that.

@elazarg
Copy link
Contributor

elazarg commented Nov 20, 2017

My bad indeed. Thanks @ilevkivskyi.
Removing explicit baseclasses works (python/typeshed#1755):

../tmp.py:7: error: Revealed type is 'builtins.frozenset[tmp.Colors*]'
../tmp.py:8: error: Revealed type is 'builtins.frozenset[tmp.Colors*]'

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

Successfully merging a pull request may close this issue.

4 participants