Skip to content

Allow E[<str>] where E is an Enum type - #2812

Merged
JukkaL merged 2 commits into
masterfrom
enum-index
Feb 7, 2017
Merged

Allow E[<str>] where E is an Enum type#2812
JukkaL merged 2 commits into
masterfrom
enum-index

Conversation

@gvanrossum

@gvanrossum gvanrossum commented Feb 6, 2017

Copy link
Copy Markdown
Member

Fixes #1381

ilevkivskyi
ilevkivskyi reviewed Feb 6, 2017
View reviewed changes
Comment thread mypy/checkexpr.py Outdated
elif isinstance(left_type, TypedDictType):
return self.visit_typeddict_index_expr(left_type, e.index)
elif (isinstance(left_type, CallableType)
and left_type.is_type_obj() and left_type.type_object()):

@ilevkivskyi ilevkivskyi Feb 6, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this already works correctly, but I think it would be safer to check for left_type.type_object().is_enum

ilevkivskyi
ilevkivskyi reviewed Feb 6, 2017
View reviewed changes
Comment thread mypy/checkexpr.py
string_type = UnionType.make_union([string_type,
self.named_type('builtins.unicode')])
self.chk.check_subtype(self.accept(index), string_type, context,
"Enum index should be a string", "actual index type")

@ilevkivskyi ilevkivskyi Feb 6, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to have a test that triggers this error.

@elazarg

elazarg commented Feb 6, 2017

Copy link
Copy Markdown
Contributor

Perhaps I am confusing things, but I think this can (and probably should?) be fixed after #2475 is merged, by augmenting EnumMeta and without further special casing.

@gvanrossum

gvanrossum commented Feb 6, 2017

Copy link
Copy Markdown
Member Author

I actually looked into basing this on #2475, but there's a catch: because of the special-casing of X[Y] for generic types, defining __getitem__ on the metaclass doesn't work -- in fact that's the one special method for which your PR doesn't work...

It might be possible to differentiate on the basis of whether X is a generic class, but that's not currently taken into account. I haven't looked into why, but perhaps it's because we need to know how to interpret X[Y] before the type of X is known?

@JukkaL
JukkaL merged commit b4492b7 into master Feb 7, 2017
@gvanrossum
gvanrossum deleted the enum-index branch February 7, 2017 17:27
@elazarg elazarg mentioned this pull request Feb 8, 2017
Merged
elazarg added a commit to elazarg/mypy that referenced this pull request Feb 8, 2017
@elazarg
partially revert python#2812
14cb040
gvanrossum pushed a commit that referenced this pull request Feb 12, 2017
@elazarg @gvanrossum
Allow using __getitem__ defined in metaclasses of non-generic classes (…
e0e9c2e 
…#2827)

Fix #1771. (Seems to be much simpler than I thought)

In particular, this will allow removing some special-casing for enum (#2812).

Related to #1381.
@JukkaL JukkaL mentioned this pull request Feb 24, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ilevkivskyi ilevkivskyi ilevkivskyi left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@gvanrossum @elazarg @ilevkivskyi @JukkaL