From 4e704d7847f2333f581f87e31b42e44a471df93a Mon Sep 17 00:00:00 2001 From: Ethan Furman Date: Mon, 25 Jul 2022 11:05:10 -0700 Subject: gh-95077: [Enum] add code-based deprecation warnings for member.member access (GH-95083) * issue deprecation warning for member.member access * always store member property in current class * remove __getattr__ --- Doc/howto/enum.rst | 19 +---- Doc/library/enum.rst | 7 -- Lib/enum.py | 86 +++++++++++----------- Lib/test/test_enum.py | 16 +++- .../2022-07-22-00-58-49.gh-issue-95077.4Z6CNC.rst | 1 + 5 files changed, 61 insertions(+), 68 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2022-07-22-00-58-49.gh-issue-95077.4Z6CNC.rst diff --git a/Doc/howto/enum.rst b/Doc/howto/enum.rst index 7b1cf75..376934a 100644 --- a/Doc/howto/enum.rst +++ b/Doc/howto/enum.rst @@ -945,23 +945,12 @@ but remain normal attributes. """""""""""""""""""" Enum members are instances of their enum class, and are normally accessed as -``EnumClass.member``. In Python versions ``3.5`` to ``3.10`` you could access -members from other members -- this practice was discouraged, and in ``3.11`` -:class:`Enum` returns to not allowing it:: - - >>> class FieldTypes(Enum): - ... name = 0 - ... value = 1 - ... size = 2 - ... - >>> FieldTypes.value.size - Traceback (most recent call last): - ... - AttributeError: member has no attribute 'size' - +``EnumClass.member``. In Python versions starting with ``3.5`` you could access +members from other members -- this practice is discouraged, is deprecated +in ``3.12``, and will be removed in ``3.14``. .. versionchanged:: 3.5 -.. versionchanged:: 3.11 +.. versionchanged:: 3.12 Creating members that are mixed with other data types diff --git a/Doc/library/enum.rst b/Doc/library/enum.rst index b1333c7..01743b0 100644 --- a/Doc/library/enum.rst +++ b/Doc/library/enum.rst @@ -176,13 +176,6 @@ Data Types >>> dir(Color) ['BLUE', 'GREEN', 'RED', '__class__', '__contains__', '__doc__', '__getitem__', '__init_subclass__', '__iter__', '__len__', '__members__', '__module__', '__name__', '__qualname__'] - .. method:: EnumType.__getattr__(cls, name) - - Returns the Enum member in *cls* matching *name*, or raises an :exc:`AttributeError`:: - - >>> Color.GREEN - - .. method:: EnumType.__getitem__(cls, name) Returns the Enum member in *cls* matching *name*, or raises an :exc:`KeyError`:: diff --git a/Lib/enum.py b/Lib/enum.py index 935e2bd..8ef6958 100644 --- a/Lib/enum.py +++ b/Lib/enum.py @@ -185,19 +185,35 @@ class property(DynamicClassAttribute): a corresponding enum member. """ + member = None + def __get__(self, instance, ownerclass=None): if instance is None: - try: - return ownerclass._member_map_[self.name] - except KeyError: + if self.member is not None: + return self.member + else: raise AttributeError( '%r has no attribute %r' % (ownerclass, self.name) ) else: if self.fget is None: - raise AttributeError( - '%r member has no attribute %r' % (ownerclass, self.name) + if self.member is None: # not sure this can happen, but just in case + raise AttributeError( + '%r has no attribute %r' % (ownerclass, self.name) + ) + # issue warning deprecating this behavior + import warnings + warnings.warn( + "`member.member` access (e.g. `Color.RED.BLUE`) is " + "deprecated and will be removed in 3.14.", + DeprecationWarning, + stacklevel=2, ) + return self.member + # XXX: uncomment in 3.14 and remove warning above + # raise AttributeError( + # '%r member has no attribute %r' % (ownerclass, self.name) + # ) else: return self.fget(instance) @@ -299,30 +315,20 @@ class _proto_member: enum_class._member_names_.append(member_name) # get redirect in place before adding to _member_map_ # but check for other instances in parent classes first - need_override = False descriptor = None for base in enum_class.__mro__[1:]: descriptor = base.__dict__.get(member_name) if descriptor is not None: if isinstance(descriptor, (property, DynamicClassAttribute)): break - else: - need_override = True - # keep looking for an enum.property - if descriptor and not need_override: - # previous enum.property found, no further action needed - pass - elif descriptor and need_override: - redirect = property() - redirect.__set_name__(enum_class, member_name) - # Previous enum.property found, but some other inherited attribute - # is in the way; copy fget, fset, fdel to this one. - redirect.fget = descriptor.fget - redirect.fset = descriptor.fset - redirect.fdel = descriptor.fdel - setattr(enum_class, member_name, redirect) - else: - setattr(enum_class, member_name, enum_member) + redirect = property() + redirect.member = enum_member + redirect.__set_name__(enum_class, member_name) + if descriptor: + redirect.fget = getattr(descriptor, 'fget', None) + redirect.fset = getattr(descriptor, 'fset', None) + redirect.fdel = getattr(descriptor, 'fdel', None) + setattr(enum_class, member_name, redirect) # now add to _member_map_ (even aliases) enum_class._member_map_[member_name] = enum_member try: @@ -740,22 +746,6 @@ class EnumType(type): # return whatever mixed-in data type has return sorted(set(dir(cls._member_type_)) | interesting) - def __getattr__(cls, name): - """ - Return the enum member matching `name` - - We use __getattr__ instead of descriptors or inserting into the enum - class' __dict__ in order to support `name` and `value` being both - properties for enum members (which live in the class' __dict__) and - enum members themselves. - """ - if _is_dunder(name): - raise AttributeError(name) - try: - return cls._member_map_[name] - except KeyError: - raise AttributeError(name) from None - def __getitem__(cls, name): """ Return the member matching `name`. @@ -1200,10 +1190,10 @@ class Enum(metaclass=EnumType): # enum.property is used to provide access to the `name` and # `value` attributes of enum members while keeping some measure of # protection from modification, while still allowing for an enumeration - # to have members named `name` and `value`. This works because enumeration - # members are not set directly on the enum class; they are kept in a - # separate structure, _member_map_, which is where enum.property looks for - # them + # to have members named `name` and `value`. This works because each + # instance of enum.property saves its companion member, which it returns + # on class lookup; on instance lookup it either executes a provided function + # or raises an AttributeError. @property def name(self): @@ -1677,10 +1667,12 @@ def _simple_enum(etype=Enum, *, boundary=None, use_args=None): value = gnv(name, 1, len(member_names), gnv_last_values) if value in value2member_map: # an alias to an existing member + member = value2member_map[value] redirect = property() + redirect.member = member redirect.__set_name__(enum_class, name) setattr(enum_class, name, redirect) - member_map[name] = value2member_map[value] + member_map[name] = member else: # create the member if use_args: @@ -1696,6 +1688,7 @@ def _simple_enum(etype=Enum, *, boundary=None, use_args=None): member.__objclass__ = enum_class member.__init__(value) redirect = property() + redirect.member = member redirect.__set_name__(enum_class, name) setattr(enum_class, name, redirect) member_map[name] = member @@ -1723,10 +1716,12 @@ def _simple_enum(etype=Enum, *, boundary=None, use_args=None): value = value.value if value in value2member_map: # an alias to an existing member + member = value2member_map[value] redirect = property() + redirect.member = member redirect.__set_name__(enum_class, name) setattr(enum_class, name, redirect) - member_map[name] = value2member_map[value] + member_map[name] = member else: # create the member if use_args: @@ -1743,6 +1738,7 @@ def _simple_enum(etype=Enum, *, boundary=None, use_args=None): member.__init__(value) member._sort_order_ = len(member_names) redirect = property() + redirect.member = member redirect.__set_name__(enum_class, name) setattr(enum_class, name, redirect) member_map[name] = member diff --git a/Lib/test/test_enum.py b/Lib/test/test_enum.py index bfab0bd..9d16fbc 100644 --- a/Lib/test/test_enum.py +++ b/Lib/test/test_enum.py @@ -2646,7 +2646,10 @@ class TestSpecial(unittest.TestCase): self.assertEqual(Private._Private__corporal, 'Radar') self.assertEqual(Private._Private__major_, 'Hoolihan') - @unittest.skip("Accessing all values retained for performance reasons, see GH-93910") + @unittest.skipIf( + python_version <= (3, 13), + 'member.member access currently deprecated', + ) def test_exception_for_member_from_member_access(self): with self.assertRaisesRegex(AttributeError, " member has no attribute .NO."): class Di(Enum): @@ -2654,6 +2657,17 @@ class TestSpecial(unittest.TestCase): NO = 0 nope = Di.YES.NO + @unittest.skipIf( + python_version > (3, 13), + 'member.member access now raises', + ) + def test_warning_for_member_from_member_access(self): + with self.assertWarnsRegex(DeprecationWarning, '`member.member` access .* is deprecated and will be removed in 3.14'): + class Di(Enum): + YES = 1 + NO = 0 + warn = Di.YES.NO + self.assertIs(warn, Di.NO) def test_dynamic_members_with_static_methods(self): # diff --git a/Misc/NEWS.d/next/Library/2022-07-22-00-58-49.gh-issue-95077.4Z6CNC.rst b/Misc/NEWS.d/next/Library/2022-07-22-00-58-49.gh-issue-95077.4Z6CNC.rst new file mode 100644 index 0000000..09f3500 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2022-07-22-00-58-49.gh-issue-95077.4Z6CNC.rst @@ -0,0 +1 @@ +Add deprecation warning for enum ``member.member`` access (e.g. ``Color.RED.BLUE``). -- cgit v0.12