From b1b539245d8d0c738444961d5a0b51eec3760db0 Mon Sep 17 00:00:00 2001 From: Ethan Furman Date: Mon, 17 Apr 2023 16:13:53 -0700 Subject: [PATCH 1/2] do not shadow mixed-in methods/attributes For example: class Book(StrEnum): title = auto() author = auto() desc = auto() Book.author.desc is Book.desc but Book.author.title() == 'Author' Using upper-case member names means this isn't an issue in practice. --- Doc/howto/enum.rst | 14 ++- Doc/library/enum.rst | 7 +- Lib/enum.py | 86 +++++++++++-------- Lib/test/test_enum.py | 17 ++++ ...-04-17-14-47-28.gh-issue-103596.ME1y3_.rst | 2 + 5 files changed, 85 insertions(+), 41 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2023-04-17-14-47-28.gh-issue-103596.ME1y3_.rst diff --git a/Doc/howto/enum.rst b/Doc/howto/enum.rst index 56391a026cf889..8d0a515e9afe05 100644 --- a/Doc/howto/enum.rst +++ b/Doc/howto/enum.rst @@ -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 betweewe 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 @@ -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 -------- @@ -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 diff --git a/Doc/library/enum.rst b/Doc/library/enum.rst index 07acf9da33e275..582e06261afd72 100644 --- a/Doc/library/enum.rst +++ b/Doc/library/enum.rst @@ -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` @@ -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: @@ -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. diff --git a/Lib/enum.py b/Lib/enum.py index e9f224a303d3e5..6e497f7ef6a7de 100644 --- a/Lib/enum.py +++ b/Lib/enum.py @@ -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: @@ -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( - " cannot set attribute %r" % (self.clsname, self.name) - ) - else: + if self.fset is not None: return self.fset(instance, value) + raise AttributeError( + " cannot set attribute %r" % (self.clsname, self.name) + ) def __delete__(self, instance): - if self.fdel is None: - raise AttributeError( - " cannot delete attribute %r" % (self.clsname, self.name) - ) - else: + if self.fdel is not None: return self.fdel(instance) + raise AttributeError( + " cannot delete attribute %r" % (self.clsname, self.name) + ) def __set_name__(self, ownerclass, name): self.name = name @@ -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) diff --git a/Lib/test/test_enum.py b/Lib/test/test_enum.py index e9dfcf8586a823..fb7a016c9007f8 100644 --- a/Lib/test/test_enum.py +++ b/Lib/test/test_enum.py @@ -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): diff --git a/Misc/NEWS.d/next/Library/2023-04-17-14-47-28.gh-issue-103596.ME1y3_.rst b/Misc/NEWS.d/next/Library/2023-04-17-14-47-28.gh-issue-103596.ME1y3_.rst new file mode 100644 index 00000000000000..2fa27e60b58efe --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-04-17-14-47-28.gh-issue-103596.ME1y3_.rst @@ -0,0 +1,2 @@ +Attributes/methods are no longer shadowed by same-named enum members, +although they may be shadowed by enum.property's. From 0d06493741aec2a8342b12313be325c91bd1cd41 Mon Sep 17 00:00:00 2001 From: Ethan Furman Date: Tue, 18 Apr 2023 09:48:42 -0700 Subject: [PATCH 2/2] fix typo Co-authored-by: samypr100 <3933065+samypr100@users.noreply.github.com> --- Doc/howto/enum.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Doc/howto/enum.rst b/Doc/howto/enum.rst index 8d0a515e9afe05..68b75c529e92c7 100644 --- a/Doc/howto/enum.rst +++ b/Doc/howto/enum.rst @@ -37,7 +37,7 @@ inherits from :class:`Enum` itself. .. note:: Case of Enum Members Because Enums are used to represent constants, and to help avoid issues - with name clashes betweewe mixin-class methods/attributes and enum names, + 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.