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__
This commit is contained in:
Ethan Furman 2022-07-25 11:05:10 -07:00 committed by GitHub
parent 73ee5a6b86
commit 4e704d7847
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 61 additions and 68 deletions

View file

@ -945,23 +945,12 @@ but remain normal attributes.
"""""""""""""""""""" """"""""""""""""""""
Enum members are instances of their enum class, and are normally accessed as 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 ``EnumClass.member``. In Python versions starting with ``3.5`` you could access
members from other members -- this practice was discouraged, and in ``3.11`` members from other members -- this practice is discouraged, is deprecated
:class:`Enum` returns to not allowing it:: in ``3.12``, and will be removed in ``3.14``.
>>> class FieldTypes(Enum):
... name = 0
... value = 1
... size = 2
...
>>> FieldTypes.value.size
Traceback (most recent call last):
...
AttributeError: <enum 'FieldTypes'> member has no attribute 'size'
.. versionchanged:: 3.5 .. versionchanged:: 3.5
.. versionchanged:: 3.11 .. versionchanged:: 3.12
Creating members that are mixed with other data types Creating members that are mixed with other data types

View file

@ -176,13 +176,6 @@ Data Types
>>> dir(Color) >>> dir(Color)
['BLUE', 'GREEN', 'RED', '__class__', '__contains__', '__doc__', '__getitem__', '__init_subclass__', '__iter__', '__len__', '__members__', '__module__', '__name__', '__qualname__'] ['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
<Color.GREEN: 2>
.. method:: EnumType.__getitem__(cls, name) .. method:: EnumType.__getitem__(cls, name)
Returns the Enum member in *cls* matching *name*, or raises an :exc:`KeyError`:: Returns the Enum member in *cls* matching *name*, or raises an :exc:`KeyError`::

View file

@ -185,19 +185,35 @@ class property(DynamicClassAttribute):
a corresponding enum member. a corresponding enum member.
""" """
member = None
def __get__(self, instance, ownerclass=None): def __get__(self, instance, ownerclass=None):
if instance is None: if instance is None:
try: if self.member is not None:
return ownerclass._member_map_[self.name] return self.member
except KeyError: else:
raise AttributeError( raise AttributeError(
'%r has no attribute %r' % (ownerclass, self.name) '%r has no attribute %r' % (ownerclass, self.name)
) )
else: else:
if self.fget is None: if self.fget is None:
raise AttributeError( if self.member is None: # not sure this can happen, but just in case
'%r member has no attribute %r' % (ownerclass, self.name) 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: else:
return self.fget(instance) return self.fget(instance)
@ -299,30 +315,20 @@ class _proto_member:
enum_class._member_names_.append(member_name) enum_class._member_names_.append(member_name)
# get redirect in place before adding to _member_map_ # get redirect in place before adding to _member_map_
# but check for other instances in parent classes first # but check for other instances in parent classes first
need_override = False
descriptor = None descriptor = None
for base in enum_class.__mro__[1:]: for base in enum_class.__mro__[1:]:
descriptor = base.__dict__.get(member_name) descriptor = base.__dict__.get(member_name)
if descriptor is not None: if descriptor is not None:
if isinstance(descriptor, (property, DynamicClassAttribute)): if isinstance(descriptor, (property, DynamicClassAttribute)):
break break
else: redirect = property()
need_override = True redirect.member = enum_member
# keep looking for an enum.property redirect.__set_name__(enum_class, member_name)
if descriptor and not need_override: if descriptor:
# previous enum.property found, no further action needed redirect.fget = getattr(descriptor, 'fget', None)
pass redirect.fset = getattr(descriptor, 'fset', None)
elif descriptor and need_override: redirect.fdel = getattr(descriptor, 'fdel', None)
redirect = property() setattr(enum_class, member_name, redirect)
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)
# now add to _member_map_ (even aliases) # now add to _member_map_ (even aliases)
enum_class._member_map_[member_name] = enum_member enum_class._member_map_[member_name] = enum_member
try: try:
@ -740,22 +746,6 @@ class EnumType(type):
# return whatever mixed-in data type has # return whatever mixed-in data type has
return sorted(set(dir(cls._member_type_)) | interesting) 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): def __getitem__(cls, name):
""" """
Return the member matching `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 # enum.property is used to provide access to the `name` and
# `value` attributes of enum members while keeping some measure of # `value` attributes of enum members while keeping some measure of
# protection from modification, while still allowing for an enumeration # protection from modification, while still allowing for an enumeration
# to have members named `name` and `value`. This works because enumeration # to have members named `name` and `value`. This works because each
# members are not set directly on the enum class; they are kept in a # instance of enum.property saves its companion member, which it returns
# separate structure, _member_map_, which is where enum.property looks for # on class lookup; on instance lookup it either executes a provided function
# them # or raises an AttributeError.
@property @property
def name(self): 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) value = gnv(name, 1, len(member_names), gnv_last_values)
if value in value2member_map: if value in value2member_map:
# an alias to an existing member # an alias to an existing member
member = value2member_map[value]
redirect = property() redirect = property()
redirect.member = member
redirect.__set_name__(enum_class, name) redirect.__set_name__(enum_class, name)
setattr(enum_class, name, redirect) setattr(enum_class, name, redirect)
member_map[name] = value2member_map[value] member_map[name] = member
else: else:
# create the member # create the member
if use_args: if use_args:
@ -1696,6 +1688,7 @@ def _simple_enum(etype=Enum, *, boundary=None, use_args=None):
member.__objclass__ = enum_class member.__objclass__ = enum_class
member.__init__(value) member.__init__(value)
redirect = property() redirect = property()
redirect.member = member
redirect.__set_name__(enum_class, name) redirect.__set_name__(enum_class, name)
setattr(enum_class, name, redirect) setattr(enum_class, name, redirect)
member_map[name] = member member_map[name] = member
@ -1723,10 +1716,12 @@ def _simple_enum(etype=Enum, *, boundary=None, use_args=None):
value = value.value value = value.value
if value in value2member_map: if value in value2member_map:
# an alias to an existing member # an alias to an existing member
member = value2member_map[value]
redirect = property() redirect = property()
redirect.member = member
redirect.__set_name__(enum_class, name) redirect.__set_name__(enum_class, name)
setattr(enum_class, name, redirect) setattr(enum_class, name, redirect)
member_map[name] = value2member_map[value] member_map[name] = member
else: else:
# create the member # create the member
if use_args: if use_args:
@ -1743,6 +1738,7 @@ def _simple_enum(etype=Enum, *, boundary=None, use_args=None):
member.__init__(value) member.__init__(value)
member._sort_order_ = len(member_names) member._sort_order_ = len(member_names)
redirect = property() redirect = property()
redirect.member = member
redirect.__set_name__(enum_class, name) redirect.__set_name__(enum_class, name)
setattr(enum_class, name, redirect) setattr(enum_class, name, redirect)
member_map[name] = member member_map[name] = member

View file

@ -2646,7 +2646,10 @@ class TestSpecial(unittest.TestCase):
self.assertEqual(Private._Private__corporal, 'Radar') self.assertEqual(Private._Private__corporal, 'Radar')
self.assertEqual(Private._Private__major_, 'Hoolihan') 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): def test_exception_for_member_from_member_access(self):
with self.assertRaisesRegex(AttributeError, "<enum .Di.> member has no attribute .NO."): with self.assertRaisesRegex(AttributeError, "<enum .Di.> member has no attribute .NO."):
class Di(Enum): class Di(Enum):
@ -2654,6 +2657,17 @@ class TestSpecial(unittest.TestCase):
NO = 0 NO = 0
nope = Di.YES.NO 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): def test_dynamic_members_with_static_methods(self):
# #

View file

@ -0,0 +1 @@
Add deprecation warning for enum ``member.member`` access (e.g. ``Color.RED.BLUE``).