Skip to content

Make Enum Iterable (etc.) only structurally #1755

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

Merged
merged 2 commits into from
Dec 21, 2017

Conversation

elazarg
Copy link
Contributor

@elazarg elazarg commented Nov 20, 2017

Should fix python/mypy#3210 and python/mypy#3349

Remove Iterable, Reversible and Container from being (nominal) baseclasses of Enum.

Can't express the precise type nominally - see mypy#3210 for details
@elazarg
Copy link
Contributor Author

elazarg commented Nov 20, 2017

How should I add a test for this?

@ilevkivskyi
Copy link
Member

There is one small warning, pytype and PyCharm may be not ready for this. (In principle it should be possible to fix this in mypy by modifying nominal constrain inference to make it more like structural, but I am not really proposing this since I don't have time to implement this in nearest future.)

@JelleZijlstra
Copy link
Member

@elazarg typeshed doesn't currently have tests for specific stubs. There is an issue for adding it. In the meantime, could you share files you tested with mypy and their output?

@matthiaskramm @vlasovskikh are you OK with this change (which depends on protocols) for mypy and pycharm?

@elazarg
Copy link
Contributor Author

elazarg commented Nov 20, 2017

[updated with bad tests]

from enum import Enum
from typing import Container, Iterable, Reversible, TypeVar
T = TypeVar('T')

class E(Enum):
    one = 1

reveal_type(list(E))  # E: Revealed type is 'builtins.list[E*]'
reveal_type(iter(E))  # E: Revealed type is 'typing.Iterator[E*]'
reveal_type(reversed(E))  # E: Revealed type is 'typing.Iterator[E*]'
reveal_type(x for x in E)  # E: Revealed type is 'typing.Iterator[E*]'
for a in E: reveal_type(a)  # E: Revealed type is 'E*'
x: Container[E] = E
xx: Container[str] = E  # doesn't fail - bad
y: Iterable[E] = E
yy: Iterable[str] = E  # doesn't fail - bad
z: Reversible[E] = E
zz: Reversible[str] = E  # doesn't fail - bad

def c(x: Container[T]) -> T: raise Exception
def it(x: Iterable[T]) -> T: raise Exception
def re(x: Reversible[T]) -> T: raise Exception
reveal_type(c(E))  # E: Revealed type is '<nothing>'  # Bad; see below
reveal_type(it(E))  # E: Revealed type is 'E'
reveal_type(re(E))  # E: Revealed type is 'E'

Container does not seem to be suitable for inference as a protocol, since the signature of __contains__ accepts Any. Maybe I should keep it as an explicit base class.

(And maybe Container should not be generic in the first place?)

@ilevkivskyi
Copy link
Member

Situation with Container is a topic of a separate issue: mypy infers UninhabitedType if there are no constraints, see python/mypy#3032

@elazarg
Copy link
Contributor Author

elazarg commented Nov 20, 2017

@JelleZijlstra given the bad tests in the updated tests, this PR should not be merged yet. I'd like to keep this open as a WIP though. I wonder why the assignments does not fail.

@elazarg elazarg changed the title Make Enum Iterable (etc.) only structurally [WIP] Make Enum Iterable (etc.) only structurally Nov 20, 2017
@elazarg
Copy link
Contributor Author

elazarg commented Nov 20, 2017

@ilevkivskyi do you have any hint as to why x: Iterable[A] = MyEnum will not fail?

@ilevkivskyi
Copy link
Member

This may be a problem in bind_self. I also realized that there is only one small test in mypy for protocols that use self-types of the form Type[T]. It makes sense to open an issue for this on mypy tracker.

@ilevkivskyi
Copy link
Member

@elazarg Yes, it looks like the fix is very simple, bind_self assumes that Type[T] can appear only in class methods, replacing is_classmethod with is_classmethod or isinstance(original_type, Instance) and original_type.type.is_metaclass() fixes the problem, I get:

Incompatible types in assignment (expression has type "Type[E]", variable has type "Iterable[str]")

@ilevkivskyi
Copy link
Member

Hm, but then I get other spurious errors, so the actual fix will be a bit more complex, but I think this is the right direction.

@elazarg
Copy link
Contributor Author

elazarg commented Nov 21, 2017

Thanks. I think I will open an issue about this.

@vlasovskikh
Copy link
Member

@JelleZijlstra PyCharm doesn't support protocols yet, but we will support them eventually, so this change is OK.

@elazarg elazarg changed the title [WIP] Make Enum Iterable (etc.) only structurally Make Enum Iterable (etc.) only structurally Dec 21, 2017
@JelleZijlstra JelleZijlstra merged commit b41c6da into python:master Dec 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Passing enum directly as iterable loses type specificity
4 participants