classification
Title: reuse of enum names in class creation inconsistent
Type: behavior Stage: resolved
Components: Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: ethan.furman Nosy List: barry, eli.bendersky, eric.snow, ethan.furman, georg.brandl, ncoghlan, pitrou, python-dev, r.david.murray
Priority: normal Keywords: patch

Created on 2013-09-09 18:58 by ethan.furman, last changed 2013-09-15 19:34 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
no_reuse_of_enum_names.stoneleaf.01.patch ethan.furman, 2013-09-09 18:58 review
Messages (17)
msg197375 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2013-09-09 18:58
Consider:

==================================================================================
-->from enum import Enum

-->class Color(Enum):
...   red = 1
...   green = 2
...   blue = 3
...   red = 4
... 
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<stdin>", line 5, in Color
  File "/home/ethan/source/python/issue18924/Lib/enum.py", line 87, in __setitem__
    raise TypeError('Attempted to reuse key: %r' % key)
TypeError: Attempted to reuse key: 'red'
==================================================================================

versus

==================================================================================
-->class Color(Enum):
...   red = 1
...   green = 2
...   blue = 3
...   def red(self):
...     return 'fooled ya!'
... 

# no error
==================================================================================

or

==================================================================================
-->class Color(Enum):
...   red = 1
...   green = 2
...   blue = 3
...   @property
...   def red(self):
...     return 'fooled ya!'
... 

# no error
==================================================================================

In both of the latter two cases the redefinition of 'red' is allowed because the new definition is not an enum member.

This is inconsistent as well as confusing.

I know normal class creation semantics don't place any such limitations on names and allow redefining at will, but Enum is not a regular class.
msg197388 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2013-09-09 19:49
As I recall this was discussed at length and we decided that this was the behavior we wanted.  I could be wrong, of course; it was a long discussion.
msg197413 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-09-09 22:36
I don't recall this *particular* wrinkle being discussed. Silently failing
to define one of the requested enum members seems quite dubious.
msg197576 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2013-09-13 13:42
I agree with David. This is yet another case where we try to go against Python and make enum special, and I'm against this. Nothing prevents users from accidentally overriding their own members and methods in normal classes as well. This is trivially discoverable with tests. Let's please stop making enum more and more complex with all these needless type checks. We'll never get out of it.
msg197591 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2013-09-13 15:31
In any other (normal) class, this works:
==================================================================================
-->class Okay:
...   red = 1
...   green = 2
...   blue = 3
...   red = 4
==================================================================================

But not in Enum.

And this works:
==================================================================================
-->class Color:
...   red = 1
...   green = 2
...   blue = 3
...   def red(self):
...     return 42
... 
--> Color.red
<property object at 0x7fee4db306d8>
==================================================================================

This only works in Enum because we added code to deal with it.  If we hadn't, we'd see this:
==================================================================================
<Color.red: <property object at 0x7fee4db30f58>>
==================================================================================

So right now the rule is:  you can overwrite an enum member with anything besides another enum member.  Considering the point of the Enum class is to create enum members this rule is nonsensical as it allows the removal of already created members.

At the very least the rule should be: enum members cannot be overwritten.

I prefer to have the rule: enum members cannot overwrite anything else, and cannot be overwritten by anything else.  This seems to me to be a simpler, more consistent rule, and more in keeping with what an enumeration should be doing.
msg197593 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-09-13 15:33
What if I redefine an existing key inside a subclass?
msg197594 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2013-09-13 15:34
One cannot subclass an Enum that has already defined keys.
msg197597 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2013-09-13 15:37
Perhaps you meant, what if you define a key in a subclass that shadows a method/property in a parent class?

I'm inclined to say that would be acceptable, since one reason for subclassing is to add or make changes to the parent class' behavior.
msg197681 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-09-14 02:11
+1 from me to just allow the names to be overwritten, even by another enum
member.
msg197685 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2013-09-14 05:06
That is expressly forbidden in the PEP.  Can we change it now?
msg197687 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2013-09-14 07:02
It can be changed up to beta1, with good reasons of course. Many PEPs are actually out of date with respect to the actual implementation in minor points.
msg197688 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2013-09-14 07:03
That said, for a PEP that has seen so much discussion before being accepted, the agreed-on API should not be changed lightly.
msg197699 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-09-14 10:13
Le samedi 14 septembre 2013 à 07:03 +0000, Georg Brandl a écrit :
> Georg Brandl added the comment:
> 
> That said, for a PEP that has seen so much discussion before being
> accepted, the agreed-on API should not be changed lightly.

I agree with that.
Also, I think it was argued that Enums are by design "restricted"
classes in that they forbid certain things in the name of convenience
and expectability. I think it is a reasonable design point and, as such,
Enums needn't be as flexible and versatile as normal classes.
msg197707 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2013-09-14 12:50
On Fri, Sep 13, 2013 at 7:11 PM, Nick Coghlan <report@bugs.python.org>wrote:

>
> Nick Coghlan added the comment:
>
> +1 from me to just allow the names to be overwritten, even by another enum
> member.
>

Even though I was in favor of this in the initial discussions (obviously,
as I'm generally in favor of Enum being less magic and special) and had to
agree with the consensus, I think it's too late now - unless we want to
reopen the pandora box. Otherwise, it would not be fair, IMHO. In the
original discussions a lot of people gave their opinions and had the
context to chime in - now changing this "quietly" in the bug tracker with
only a handful of participants isn't appropriate.

The current behavior is fine, I think. The vast majority of enums will not
have methods, and some amount of safety against typos in member definitions
makes sense. Methods have other rules anyway (such as being definable in
subclasses), and since their uses are rarer and arguably more deliberate,
leaving them with more a Pythonic nature should be fine. This behavior is
documented in the PEP and documentation and is fairly well understood.
Also, we can always lift restrictions later without breaking existing code,
if it's deemed that some restrictions are too much.
msg197717 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-09-14 16:17
OK, rechecking PEP 435, I see that disallowing reuse of a name was indeed explicitly accepted as part of the defined API: http://www.python.org/dev/peps/pep-0435/#duplicating-enum-members-and-values

In that case, I switch my perspective to agree with Ethan that overwriting it with a method or descriptor should *also* be disallowed. The PEP is currently silent on that question, and as Ethan notes in the original post, the weird middle ground of the current behaviour is thoroughly confusing:

>>> class Disallowed(Enum):
...     a = 1
...     a = 2
... 
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<stdin>", line 3, in Disallowed
  File "/home/ncoghlan/devel/py3k/Lib/enum.py", line 87, in __setitem__
    raise TypeError('Attempted to reuse key: %r' % key)
TypeError: Attempted to reuse key: 'a'


>>> class Allowed(Enum):
...     a = 1
...     @property
...     def a(self):
...         return 2
... 
>>> Allowed.a
<property object at 0x7f3d65da14d8>
>>> Allowed().a
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: __call__() missing 1 required positional argument: 'value'
>>> Allowed(1).a
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/ncoghlan/devel/py3k/Lib/enum.py", line 218, in __call__
    return cls.__new__(cls, value)
  File "/home/ncoghlan/devel/py3k/Lib/enum.py", line 439, in __new__
    raise ValueError("%s is not a valid %s" % (value, cls.__name__))
ValueError: 1 is not a valid Allowed
>>> Allowed('a').a
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/ncoghlan/devel/py3k/Lib/enum.py", line 218, in __call__
    return cls.__new__(cls, value)
  File "/home/ncoghlan/devel/py3k/Lib/enum.py", line 439, in __new__
    raise ValueError("%s is not a valid %s" % (value, cls.__name__))
ValueError: a is not a valid Allowed
>>> Allowed['a'].a
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/ncoghlan/devel/py3k/Lib/enum.py", line 255, in __getitem__
    return cls._member_map_[name]
KeyError: 'a'
msg197819 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2013-09-15 19:29
As David noted, this was discussed somewhere in the megathreads that was the Enum PEP discussion.

As Georg noted changes to such a thoroughly discussed API should not be taken lightly.

As Antoine noted the Enum class is more restricted than normal classes by design.

As Nick seconded the current situation is thoroughly confusing.
msg197821 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-09-15 19:34
New changeset 54ddd1124df8 by Ethan Furman in branch 'default':
Close #18989: enum members will no longer overwrite other attributes, nor be overwritten by them.
http://hg.python.org/cpython/rev/54ddd1124df8
History
Date User Action Args
2013-09-15 19:34:57python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg197821

resolution: fixed
stage: resolved
2013-09-15 19:29:07ethan.furmansetmessages: + msg197819
2013-09-14 16:17:37ncoghlansetmessages: + msg197717
2013-09-14 12:50:05eli.benderskysetmessages: + msg197707
2013-09-14 10:13:24pitrousetmessages: + msg197699
2013-09-14 07:03:22georg.brandlsetmessages: + msg197688
2013-09-14 07:02:31georg.brandlsetnosy: + georg.brandl
messages: + msg197687
2013-09-14 05:06:55ethan.furmansetmessages: + msg197685
2013-09-14 02:11:52ncoghlansetmessages: + msg197681
2013-09-13 15:37:07ethan.furmansetmessages: + msg197597
2013-09-13 15:34:14ethan.furmansetmessages: + msg197594
2013-09-13 15:33:06pitrousetmessages: + msg197593
2013-09-13 15:31:12ethan.furmansetmessages: + msg197591
2013-09-13 13:42:30eli.benderskysetmessages: + msg197576
2013-09-09 22:36:54ncoghlansetmessages: + msg197413
2013-09-09 19:49:43r.david.murraysetnosy: + r.david.murray
messages: + msg197388
2013-09-09 18:58:50ethan.furmancreate