From 3515dcce80ca51d29a7100ce0c2ac2a50ba46af8 Mon Sep 17 00:00:00 2001 From: Ethan Furman Date: Sun, 18 Sep 2016 13:15:41 -0700 Subject: [PATCH] issue23591: fix flag decomposition and repr --- Doc/library/enum.rst | 22 ++++++ Lib/enum.py | 152 ++++++++++++++++++++++++++---------------- Lib/test/test_enum.py | 105 +++++++++++++++++++++-------- 3 files changed, 193 insertions(+), 86 deletions(-) diff --git a/Doc/library/enum.rst b/Doc/library/enum.rst index eb8b94b3751..87aa8b140bc 100644 --- a/Doc/library/enum.rst +++ b/Doc/library/enum.rst @@ -674,6 +674,8 @@ while combinations of flags won't:: ... green = auto() ... white = red | blue | green ... + >>> Color.white + Giving a name to the "no flags set" condition does not change its boolean value:: @@ -1068,3 +1070,23 @@ but not of the class:: >>> dir(Planet.EARTH) ['__class__', '__doc__', '__module__', 'name', 'surface_gravity', 'value'] + +Combining members of ``Flag`` +""""""""""""""""""""""""""""" + +If a combination of Flag members is not named, the :func:`repr` will include +all named flags and all named combinations of flags that are in the value:: + + >>> class Color(Flag): + ... red = auto() + ... green = auto() + ... blue = auto() + ... magenta = red | blue + ... yellow = red | green + ... cyan = green | blue + ... + >>> Color(3) # named combination + + >>> Color(7) # not named combination + + diff --git a/Lib/enum.py b/Lib/enum.py index d8303204ee8..4beb187a4ad 100644 --- a/Lib/enum.py +++ b/Lib/enum.py @@ -1,7 +1,7 @@ import sys from types import MappingProxyType, DynamicClassAttribute from functools import reduce -from operator import or_ as _or_ +from operator import or_ as _or_, and_ as _and_, xor, neg # try _collections first to reduce startup cost try: @@ -47,11 +47,12 @@ def _make_class_unpicklable(cls): cls.__reduce_ex__ = _break_on_call_reduce cls.__module__ = '' +_auto_null = object() class auto: """ Instances are replaced with an appropriate value in Enum class suites. """ - pass + value = _auto_null class _EnumDict(dict): @@ -77,7 +78,7 @@ class _EnumDict(dict): """ if _is_sunder(key): if key not in ( - '_order_', '_create_pseudo_member_', '_decompose_', + '_order_', '_create_pseudo_member_', '_generate_next_value_', '_missing_', ): raise ValueError('_names_ are reserved for future Enum use') @@ -94,7 +95,9 @@ class _EnumDict(dict): # enum overwriting a descriptor? raise TypeError('%r already defined as: %r' % (key, self[key])) if isinstance(value, auto): - value = self._generate_next_value(key, 1, len(self._member_names), self._last_values[:]) + if value.value == _auto_null: + value.value = self._generate_next_value(key, 1, len(self._member_names), self._last_values[:]) + value = value.value self._member_names.append(key) self._last_values.append(value) super().__setitem__(key, value) @@ -658,7 +661,7 @@ class Flag(Enum): try: high_bit = _high_bit(last_value) break - except TypeError: + except Exception: raise TypeError('Invalid Flag value: %r' % last_value) from None return 2 ** (high_bit+1) @@ -668,61 +671,38 @@ class Flag(Enum): if value < 0: value = ~value possible_member = cls._create_pseudo_member_(value) - for member in possible_member._decompose_(): - if member._name_ is None and member._value_ != 0: - raise ValueError('%r is not a valid %s' % (original_value, cls.__name__)) if original_value < 0: possible_member = ~possible_member return possible_member @classmethod def _create_pseudo_member_(cls, value): + """ + Create a composite member iff value contains only members. + """ pseudo_member = cls._value2member_map_.get(value, None) if pseudo_member is None: - # construct a non-singleton enum pseudo-member + # verify all bits are accounted for + _, extra_flags = _decompose(cls, value) + if extra_flags: + raise ValueError("%r is not a valid %s" % (value, cls.__name__)) + # construct a singleton enum pseudo-member pseudo_member = object.__new__(cls) pseudo_member._name_ = None pseudo_member._value_ = value cls._value2member_map_[value] = pseudo_member return pseudo_member - def _decompose_(self): - """Extract all members from the value.""" - value = self._value_ - members = [] - cls = self.__class__ - for member in sorted(cls, key=lambda m: m._value_, reverse=True): - while _high_bit(value) > _high_bit(member._value_): - unknown = self._create_pseudo_member_(2 ** _high_bit(value)) - members.append(unknown) - value &= ~unknown._value_ - if ( - (value & member._value_ == member._value_) - and (member._value_ or not members) - ): - value &= ~member._value_ - members.append(member) - if not members or value: - members.append(self._create_pseudo_member_(value)) - members = list(members) - return members - def __contains__(self, other): if not isinstance(other, self.__class__): return NotImplemented return other._value_ & self._value_ == other._value_ - def __iter__(self): - if self.value == 0: - return iter([]) - else: - return iter(self._decompose_()) - def __repr__(self): cls = self.__class__ if self._name_ is not None: return '<%s.%s: %r>' % (cls.__name__, self._name_, self._value_) - members = self._decompose_() + members, uncovered = _decompose(cls, self._value_) return '<%s.%s: %r>' % ( cls.__name__, '|'.join([str(m._name_ or m._value_) for m in members]), @@ -733,7 +713,7 @@ class Flag(Enum): cls = self.__class__ if self._name_ is not None: return '%s.%s' % (cls.__name__, self._name_) - members = self._decompose_() + members, uncovered = _decompose(cls, self._value_) if len(members) == 1 and members[0]._name_ is None: return '%s.%r' % (cls.__name__, members[0]._value_) else: @@ -761,8 +741,11 @@ class Flag(Enum): return self.__class__(self._value_ ^ other._value_) def __invert__(self): - members = self._decompose_() - inverted_members = [m for m in self.__class__ if m not in members and not m._value_ & self._value_] + members, uncovered = _decompose(self.__class__, self._value_) + inverted_members = [ + m for m in self.__class__ + if m not in members and not m._value_ & self._value_ + ] inverted = reduce(_or_, inverted_members, self.__class__(0)) return self.__class__(inverted) @@ -770,26 +753,46 @@ class Flag(Enum): class IntFlag(int, Flag): """Support for integer-based Flags""" + @classmethod + def _missing_(cls, value): + if not isinstance(value, int): + raise ValueError("%r is not a valid %s" % (value, cls.__name__)) + new_member = cls._create_pseudo_member_(value) + return new_member + @classmethod def _create_pseudo_member_(cls, value): pseudo_member = cls._value2member_map_.get(value, None) if pseudo_member is None: - # construct a non-singleton enum pseudo-member - pseudo_member = int.__new__(cls, value) - pseudo_member._name_ = None - pseudo_member._value_ = value - cls._value2member_map_[value] = pseudo_member + need_to_create = [value] + # get unaccounted for bits + _, extra_flags = _decompose(cls, value) + # timer = 10 + while extra_flags: + # timer -= 1 + bit = _high_bit(extra_flags) + flag_value = 2 ** bit + if (flag_value not in cls._value2member_map_ and + flag_value not in need_to_create + ): + need_to_create.append(flag_value) + if extra_flags == -flag_value: + extra_flags = 0 + else: + extra_flags ^= flag_value + for value in reversed(need_to_create): + # construct singleton pseudo-members + pseudo_member = int.__new__(cls, value) + pseudo_member._name_ = None + pseudo_member._value_ = value + cls._value2member_map_[value] = pseudo_member return pseudo_member - @classmethod - def _missing_(cls, value): - possible_member = cls._create_pseudo_member_(value) - return possible_member - def __or__(self, other): if not isinstance(other, (self.__class__, int)): return NotImplemented - return self.__class__(self._value_ | self.__class__(other)._value_) + result = self.__class__(self._value_ | self.__class__(other)._value_) + return result def __and__(self, other): if not isinstance(other, (self.__class__, int)): @@ -806,17 +809,13 @@ class IntFlag(int, Flag): __rxor__ = __xor__ def __invert__(self): - # members = self._decompose_() - # inverted_members = [m for m in self.__class__ if m not in members and not m._value_ & self._value_] - # inverted = reduce(_or_, inverted_members, self.__class__(0)) - return self.__class__(~self._value_) - - + result = self.__class__(~self._value_) + return result def _high_bit(value): """returns index of highest bit, or -1 if value is zero or negative""" - return value.bit_length() - 1 if value > 0 else -1 + return value.bit_length() - 1 def unique(enumeration): """Class decorator for enumerations ensuring unique member values.""" @@ -830,3 +829,40 @@ def unique(enumeration): raise ValueError('duplicate values found in %r: %s' % (enumeration, alias_details)) return enumeration + +def _decompose(flag, value): + """Extract all members from the value.""" + # _decompose is only called if the value is not named + not_covered = value + negative = value < 0 + if negative: + # only check for named flags + flags_to_check = [ + (m, v) + for v, m in flag._value2member_map_.items() + if m.name is not None + ] + else: + # check for named flags and powers-of-two flags + flags_to_check = [ + (m, v) + for v, m in flag._value2member_map_.items() + if m.name is not None or _power_of_two(v) + ] + members = [] + for member, member_value in flags_to_check: + if member_value and member_value & value == member_value: + members.append(member) + not_covered &= ~member_value + if not members and value in flag._value2member_map_: + members.append(flag._value2member_map_[value]) + members.sort(key=lambda m: m._value_, reverse=True) + if len(members) > 1 and members[0].value == value: + # we have the breakdown, don't need the value member itself + members.pop(0) + return members, not_covered + +def _power_of_two(value): + if value < 1: + return False + return value == 2 ** _high_bit(value) diff --git a/Lib/test/test_enum.py b/Lib/test/test_enum.py index 153bfb40a79..2b3bfea9162 100644 --- a/Lib/test/test_enum.py +++ b/Lib/test/test_enum.py @@ -1634,6 +1634,13 @@ class TestEnum(unittest.TestCase): self.assertEqual(Color.blue.value, 2) self.assertEqual(Color.green.value, 3) + def test_duplicate_auto(self): + class Dupes(Enum): + first = primero = auto() + second = auto() + third = auto() + self.assertEqual([Dupes.first, Dupes.second, Dupes.third], list(Dupes)) + class TestOrder(unittest.TestCase): @@ -1731,7 +1738,7 @@ class TestFlag(unittest.TestCase): self.assertEqual(str(Open.AC), 'Open.AC') self.assertEqual(str(Open.RO | Open.CE), 'Open.CE') self.assertEqual(str(Open.WO | Open.CE), 'Open.CE|WO') - self.assertEqual(str(~Open.RO), 'Open.CE|AC') + self.assertEqual(str(~Open.RO), 'Open.CE|AC|RW|WO') self.assertEqual(str(~Open.WO), 'Open.CE|RW') self.assertEqual(str(~Open.AC), 'Open.CE') self.assertEqual(str(~(Open.RO | Open.CE)), 'Open.AC') @@ -1758,7 +1765,7 @@ class TestFlag(unittest.TestCase): self.assertEqual(repr(Open.AC), '') self.assertEqual(repr(Open.RO | Open.CE), '') self.assertEqual(repr(Open.WO | Open.CE), '') - self.assertEqual(repr(~Open.RO), '') + self.assertEqual(repr(~Open.RO), '') self.assertEqual(repr(~Open.WO), '') self.assertEqual(repr(~Open.AC), '') self.assertEqual(repr(~(Open.RO | Open.CE)), '') @@ -1949,6 +1956,33 @@ class TestFlag(unittest.TestCase): red = 'not an int' blue = auto() + def test_cascading_failure(self): + class Bizarre(Flag): + c = 3 + d = 4 + f = 6 + # Bizarre.c | Bizarre.d + self.assertRaisesRegex(ValueError, "5 is not a valid Bizarre", Bizarre, 5) + self.assertRaisesRegex(ValueError, "5 is not a valid Bizarre", Bizarre, 5) + self.assertRaisesRegex(ValueError, "2 is not a valid Bizarre", Bizarre, 2) + self.assertRaisesRegex(ValueError, "2 is not a valid Bizarre", Bizarre, 2) + self.assertRaisesRegex(ValueError, "1 is not a valid Bizarre", Bizarre, 1) + self.assertRaisesRegex(ValueError, "1 is not a valid Bizarre", Bizarre, 1) + + def test_duplicate_auto(self): + class Dupes(Enum): + first = primero = auto() + second = auto() + third = auto() + self.assertEqual([Dupes.first, Dupes.second, Dupes.third], list(Dupes)) + + def test_bizarre(self): + class Bizarre(Flag): + b = 3 + c = 4 + d = 6 + self.assertEqual(repr(Bizarre(7)), '') + class TestIntFlag(unittest.TestCase): """Tests of the IntFlags.""" @@ -1965,6 +1999,21 @@ class TestIntFlag(unittest.TestCase): AC = 3 CE = 1<<19 + def test_type(self): + Perm = self.Perm + Open = self.Open + for f in Perm: + self.assertTrue(isinstance(f, Perm)) + self.assertEqual(f, f.value) + self.assertTrue(isinstance(Perm.W | Perm.X, Perm)) + self.assertEqual(Perm.W | Perm.X, 3) + for f in Open: + self.assertTrue(isinstance(f, Open)) + self.assertEqual(f, f.value) + self.assertTrue(isinstance(Open.WO | Open.RW, Open)) + self.assertEqual(Open.WO | Open.RW, 3) + + def test_str(self): Perm = self.Perm self.assertEqual(str(Perm.R), 'Perm.R') @@ -1975,14 +2024,14 @@ class TestIntFlag(unittest.TestCase): self.assertEqual(str(Perm.R | 8), 'Perm.8|R') self.assertEqual(str(Perm(0)), 'Perm.0') self.assertEqual(str(Perm(8)), 'Perm.8') - self.assertEqual(str(~Perm.R), 'Perm.W|X|-8') - self.assertEqual(str(~Perm.W), 'Perm.R|X|-8') - self.assertEqual(str(~Perm.X), 'Perm.R|W|-8') - self.assertEqual(str(~(Perm.R | Perm.W)), 'Perm.X|-8') + self.assertEqual(str(~Perm.R), 'Perm.W|X') + self.assertEqual(str(~Perm.W), 'Perm.R|X') + self.assertEqual(str(~Perm.X), 'Perm.R|W') + self.assertEqual(str(~(Perm.R | Perm.W)), 'Perm.X') self.assertEqual(str(~(Perm.R | Perm.W | Perm.X)), 'Perm.-8') - self.assertEqual(str(~(Perm.R | 8)), 'Perm.W|X|-16') - self.assertEqual(str(Perm(~0)), 'Perm.R|W|X|-8') - self.assertEqual(str(Perm(~8)), 'Perm.R|W|X|-16') + self.assertEqual(str(~(Perm.R | 8)), 'Perm.W|X') + self.assertEqual(str(Perm(~0)), 'Perm.R|W|X') + self.assertEqual(str(Perm(~8)), 'Perm.R|W|X') Open = self.Open self.assertEqual(str(Open.RO), 'Open.RO') @@ -1991,12 +2040,12 @@ class TestIntFlag(unittest.TestCase): self.assertEqual(str(Open.RO | Open.CE), 'Open.CE') self.assertEqual(str(Open.WO | Open.CE), 'Open.CE|WO') self.assertEqual(str(Open(4)), 'Open.4') - self.assertEqual(str(~Open.RO), 'Open.CE|AC|-524292') - self.assertEqual(str(~Open.WO), 'Open.CE|RW|-524292') - self.assertEqual(str(~Open.AC), 'Open.CE|-524292') - self.assertEqual(str(~(Open.RO | Open.CE)), 'Open.AC|-524292') - self.assertEqual(str(~(Open.WO | Open.CE)), 'Open.RW|-524292') - self.assertEqual(str(Open(~4)), 'Open.CE|AC|-524296') + self.assertEqual(str(~Open.RO), 'Open.CE|AC|RW|WO') + self.assertEqual(str(~Open.WO), 'Open.CE|RW') + self.assertEqual(str(~Open.AC), 'Open.CE') + self.assertEqual(str(~(Open.RO | Open.CE)), 'Open.AC|RW|WO') + self.assertEqual(str(~(Open.WO | Open.CE)), 'Open.RW') + self.assertEqual(str(Open(~4)), 'Open.CE|AC|RW|WO') def test_repr(self): Perm = self.Perm @@ -2008,14 +2057,14 @@ class TestIntFlag(unittest.TestCase): self.assertEqual(repr(Perm.R | 8), '') self.assertEqual(repr(Perm(0)), '') self.assertEqual(repr(Perm(8)), '') - self.assertEqual(repr(~Perm.R), '') - self.assertEqual(repr(~Perm.W), '') - self.assertEqual(repr(~Perm.X), '') - self.assertEqual(repr(~(Perm.R | Perm.W)), '') + self.assertEqual(repr(~Perm.R), '') + self.assertEqual(repr(~Perm.W), '') + self.assertEqual(repr(~Perm.X), '') + self.assertEqual(repr(~(Perm.R | Perm.W)), '') self.assertEqual(repr(~(Perm.R | Perm.W | Perm.X)), '') - self.assertEqual(repr(~(Perm.R | 8)), '') - self.assertEqual(repr(Perm(~0)), '') - self.assertEqual(repr(Perm(~8)), '') + self.assertEqual(repr(~(Perm.R | 8)), '') + self.assertEqual(repr(Perm(~0)), '') + self.assertEqual(repr(Perm(~8)), '') Open = self.Open self.assertEqual(repr(Open.RO), '') @@ -2024,12 +2073,12 @@ class TestIntFlag(unittest.TestCase): self.assertEqual(repr(Open.RO | Open.CE), '') self.assertEqual(repr(Open.WO | Open.CE), '') self.assertEqual(repr(Open(4)), '') - self.assertEqual(repr(~Open.RO), '') - self.assertEqual(repr(~Open.WO), '') - self.assertEqual(repr(~Open.AC), '') - self.assertEqual(repr(~(Open.RO | Open.CE)), '') - self.assertEqual(repr(~(Open.WO | Open.CE)), '') - self.assertEqual(repr(Open(~4)), '') + self.assertEqual(repr(~Open.RO), '') + self.assertEqual(repr(~Open.WO), '') + self.assertEqual(repr(~Open.AC), '') + self.assertEqual(repr(~(Open.RO | Open.CE)), '') + self.assertEqual(repr(~(Open.WO | Open.CE)), '') + self.assertEqual(repr(Open(~4)), '') def test_or(self): Perm = self.Perm