Support overriding dunder attributes in Enum subclass #12138
Conversation
mypy/checker.py
Outdated
| if (isinstance(sym.node, Var) and sym.node.has_explicit_value and | ||
| sym.node.name == '__members__'): | ||
| self.fail( | ||
| 'Cannot declare read-only attribute "__members__"', sym.node |
There was a problem hiding this comment.
Can you please move this string to message_registry.py?
There was a problem hiding this comment.
Done, had a bit of a hard time coming up with a reasonable name for the variable. Let me know if you got any better than what I could come up with
| if defn.info.fullname not in ENUM_BASES: | ||
| for sym in defn.info.names.values(): | ||
| if (isinstance(sym.node, Var) and sym.node.has_explicit_value and | ||
| sym.node.name == '__members__'): |
There was a problem hiding this comment.
Let's add a comment: why don't we allow __members__
There was a problem hiding this comment.
So, to clarify:
>>> import enum
>>> class E(enum.Enum):
... __members__ = (1, 2)
...
>>> class D(E):
... __members__ = (3, 4)
...
>>> D.__members__
mappingproxy({})It is not a runtime error. But, it does not make sense, right?
There was a problem hiding this comment.
Yeah, exactly. __members__ will always be overwritten. So it doesn't make sense to try to declare any value to it.
This comment has been minimized.
This comment has been minimized.
test-data/unit/check-enum.test
Outdated
| from enum import Enum | ||
|
|
||
| class WritingMembers(Enum): | ||
| __members__: typing.Dict[Enum, Enum] = {} # E: Cannot declare read-only attribute "__members__" |
There was a problem hiding this comment.
I think we can improve this error message. __members__ is not read only, it more like "internal"?..
I would be quite lost with the current one.
There was a problem hiding this comment.
Yeah, agreed, I tried to generalise what's said in the documentation a bit: https://docs.python.org/3/library/enum.html#supported-dunder-names
__members__is a read-only ordered mapping ofmember_name:memberitems. It is only available on the class.
Inlining it here just for reference
What do you think about Cannot assign to internal attribute "__members__"? Not completely sure if "assign" is most appropriate though
There was a problem hiding this comment.
Maybe exposing the internal implementation would be a good idea in this case?
Like 'Assigned "__members__" will be overriden by "Enum" internally'?
There was a problem hiding this comment.
That reads good IMO, lets stick with that!
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Sorry for the slow response, can you fix the merge conflicts so that I can do another review pass and get this merged? |
de303eb to
360fd99
Compare
|
No worries, rebased and squashed just now |
|
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
|
@JukkaL friendly ping in case you missed that I bumped the branch |
Allows any dunder (`__name__`) attributes except `__members__` (due to it being read-only) to be overridden in enum subclasses. Fixes #12132.
Description
Allows any dunder(
__name__) attributes except__members__(due to it being read-only) to be overridden in enum subclasses.Fixes #12132
Test Plan
Added tests to the
check-enumsuiteExtra notes
I'm quite new to mypy's code. So please help out with stuff I might've missed or so. I'm also not sure if I've placed the
__members__fail correctly or if it's better placed in the analyser.I also tried to fix sunder (
_name_) attributes raisingAttributeErrors, but my changes needs a bit more polishing, so I left it for a later PR. Invalids there will raise during runtime anyways.