Skip to content

Commit

Permalink
gh-103596: [Enum] do not shadow mixed-in methods/attributes (GH-103600)
Browse files Browse the repository at this point in the history
For example:

    class Book(StrEnum):
        title = auto()
        author = auto()
        desc = auto()

    Book.author.desc is Book.desc

but

    Book.author.title() == 'Author'

is commonly expected.  Using upper-case member names avoids this confusion and possible performance impacts.

Co-authored-by: samypr100 <3933065+samypr100@users.noreply.github.com>
  • Loading branch information
ethanfurman and samypr100 authored Apr 18, 2023
1 parent 07804ce commit 700ec65
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 41 deletions.
14 changes: 11 additions & 3 deletions Doc/howto/enum.rst
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,10 @@ inherits from :class:`Enum` itself.

.. note:: Case of Enum Members

Because Enums are used to represent constants we recommend using
UPPER_CASE names for members, and will be using that style in our examples.
Because Enums are used to represent constants, and to help avoid issues
with name clashes between mixin-class methods/attributes and enum names,
we strongly recommend using UPPER_CASE names for members, and will be using
that style in our examples.

Depending on the nature of the enum a member's value may or may not be
important, but either way that value can be used to get the corresponding
Expand Down Expand Up @@ -490,6 +492,10 @@ the :meth:`~Enum.__repr__` omits the inherited class' name. For example::
Use the :func:`!dataclass` argument ``repr=False``
to use the standard :func:`repr`.

.. versionchanged:: 3.12
Only the dataclass fields are shown in the value area, not the dataclass'
name.


Pickling
--------
Expand Down Expand Up @@ -992,7 +998,9 @@ but remain normal attributes.
Enum members are instances of their enum class, and are normally accessed as
``EnumClass.member``. In certain situations, such as writing custom enum
behavior, being able to access one member directly from another is useful,
and is supported.
and is supported; however, in order to avoid name clashes between member names
and attributes/methods from mixed-in classes, upper-case names are strongly
recommended.

.. versionchanged:: 3.5

Expand Down
7 changes: 4 additions & 3 deletions Doc/library/enum.rst
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,8 @@ Module Contents
:func:`~enum.property`

Allows :class:`Enum` members to have attributes without conflicting with
member names.
member names. The ``value`` and ``name`` attributes are implemented this
way.

:func:`unique`

Expand Down Expand Up @@ -169,7 +170,7 @@ Data Types
final *enum*, as well as creating the enum members, properly handling
duplicates, providing iteration over the enum class, etc.

.. method:: EnumType.__call__(cls, value, names=None, *, module=None, qualname=None, type=None, start=1, boundary=None)
.. method:: EnumType.__call__(cls, value, names=None, \*, module=None, qualname=None, type=None, start=1, boundary=None)

This method is called in two different ways:

Expand Down Expand Up @@ -317,7 +318,7 @@ Data Types
>>> PowersOfThree.SECOND.value
9

.. method:: Enum.__init_subclass__(cls, **kwds)
.. method:: Enum.__init_subclass__(cls, \**kwds)

A *classmethod* that is used to further configure subsequent subclasses.
By default, does nothing.
Expand Down
86 changes: 51 additions & 35 deletions Lib/enum.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,8 @@ class property(DynamicClassAttribute):
"""

member = None
_attr_type = None
_cls_type = None

def __get__(self, instance, ownerclass=None):
if instance is None:
Expand All @@ -199,33 +201,36 @@ def __get__(self, instance, ownerclass=None):
raise AttributeError(
'%r has no attribute %r' % (ownerclass, self.name)
)
else:
if self.fget is None:
# look for a member by this name.
try:
return ownerclass._member_map_[self.name]
except KeyError:
raise AttributeError(
'%r has no attribute %r' % (ownerclass, self.name)
) from None
else:
return self.fget(instance)
if self.fget is not None:
# use previous enum.property
return self.fget(instance)
elif self._attr_type == 'attr':
# look up previous attibute
return getattr(self._cls_type, self.name)
elif self._attr_type == 'desc':
# use previous descriptor
return getattr(instance._value_, self.name)
# look for a member by this name.
try:
return ownerclass._member_map_[self.name]
except KeyError:
raise AttributeError(
'%r has no attribute %r' % (ownerclass, self.name)
) from None

def __set__(self, instance, value):
if self.fset is None:
raise AttributeError(
"<enum %r> cannot set attribute %r" % (self.clsname, self.name)
)
else:
if self.fset is not None:
return self.fset(instance, value)
raise AttributeError(
"<enum %r> cannot set attribute %r" % (self.clsname, self.name)
)

def __delete__(self, instance):
if self.fdel is None:
raise AttributeError(
"<enum %r> cannot delete attribute %r" % (self.clsname, self.name)
)
else:
if self.fdel is not None:
return self.fdel(instance)
raise AttributeError(
"<enum %r> cannot delete attribute %r" % (self.clsname, self.name)
)

def __set_name__(self, ownerclass, name):
self.name = name
Expand Down Expand Up @@ -313,27 +318,38 @@ def __set_name__(self, enum_class, member_name):
enum_class._member_names_.append(member_name)
# if necessary, get redirect in place and then add it to _member_map_
found_descriptor = None
descriptor_type = None
class_type = None
for base in enum_class.__mro__[1:]:
descriptor = base.__dict__.get(member_name)
if descriptor is not None:
if isinstance(descriptor, (property, DynamicClassAttribute)):
found_descriptor = descriptor
attr = base.__dict__.get(member_name)
if attr is not None:
if isinstance(attr, (property, DynamicClassAttribute)):
found_descriptor = attr
class_type = base
descriptor_type = 'enum'
break
elif (
hasattr(descriptor, 'fget') and
hasattr(descriptor, 'fset') and
hasattr(descriptor, 'fdel')
):
found_descriptor = descriptor
elif _is_descriptor(attr):
found_descriptor = attr
descriptor_type = descriptor_type or 'desc'
class_type = class_type or base
continue
else:
descriptor_type = 'attr'
class_type = base
if found_descriptor:
redirect = property()
redirect.member = enum_member
redirect.__set_name__(enum_class, member_name)
# earlier descriptor found; copy fget, fset, fdel to this one.
redirect.fget = found_descriptor.fget
redirect.fset = found_descriptor.fset
redirect.fdel = found_descriptor.fdel
if descriptor_type in ('enum','desc'):
# earlier descriptor found; copy fget, fset, fdel to this one.
redirect.fget = getattr(found_descriptor, 'fget', None)
redirect._get = getattr(found_descriptor, '__get__', None)
redirect.fset = getattr(found_descriptor, 'fset', None)
redirect._set = getattr(found_descriptor, '__set__', None)
redirect.fdel = getattr(found_descriptor, 'fdel', None)
redirect._del = getattr(found_descriptor, '__delete__', None)
redirect._attr_type = descriptor_type
redirect._cls_type = class_type
setattr(enum_class, member_name, redirect)
else:
setattr(enum_class, member_name, enum_member)
Expand Down
17 changes: 17 additions & 0 deletions Lib/test/test_enum.py
Original file line number Diff line number Diff line change
Expand Up @@ -819,10 +819,27 @@ class TestPlainFlag(_EnumTests, _PlainOutputTests, _FlagTests, unittest.TestCase

class TestIntEnum(_EnumTests, _MinimalOutputTests, unittest.TestCase):
enum_type = IntEnum
#
def test_shadowed_attr(self):
class Number(IntEnum):
divisor = 1
numerator = 2
#
self.assertEqual(Number.divisor.numerator, 1)
self.assertIs(Number.numerator.divisor, Number.divisor)


class TestStrEnum(_EnumTests, _MinimalOutputTests, unittest.TestCase):
enum_type = StrEnum
#
def test_shadowed_attr(self):
class Book(StrEnum):
author = 'author'
title = 'title'
#
self.assertEqual(Book.author.title(), 'Author')
self.assertEqual(Book.title.title(), 'Title')
self.assertIs(Book.title.author, Book.author)


class TestIntFlag(_EnumTests, _MinimalOutputTests, _FlagTests, unittest.TestCase):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Attributes/methods are no longer shadowed by same-named enum members,
although they may be shadowed by enum.property's.

0 comments on commit 700ec65

Please sign in to comment.