-
-
Notifications
You must be signed in to change notification settings - Fork 34.8k
bpo-44242: [Enum] remove missing bits test from Flag creation #26586
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,10 +6,10 @@ | |
| __all__ = [ | ||
| 'EnumType', 'EnumMeta', | ||
| 'Enum', 'IntEnum', 'StrEnum', 'Flag', 'IntFlag', | ||
| 'auto', 'unique', | ||
| 'property', | ||
| 'auto', 'unique', 'property', 'verify', | ||
| 'FlagBoundary', 'STRICT', 'CONFORM', 'EJECT', 'KEEP', | ||
| 'global_flag_repr', 'global_enum_repr', 'global_enum', | ||
| 'EnumCheck', 'CONTINUOUS', 'NAMED_FLAGS', 'UNIQUE', | ||
| ] | ||
|
|
||
|
|
||
|
|
@@ -89,6 +89,9 @@ def _break_on_call_reduce(self, proto): | |
| setattr(obj, '__module__', '<unknown>') | ||
|
|
||
| def _iter_bits_lsb(num): | ||
| # num must be an integer | ||
| if isinstance(num, Enum): | ||
| num = num.value | ||
| while num: | ||
| b = num & (~num + 1) | ||
| yield b | ||
|
|
@@ -538,13 +541,6 @@ def __new__(metacls, cls, bases, classdict, *, boundary=None, _simple=False, **k | |
| else: | ||
| # multi-bit flags are considered aliases | ||
| multi_bit_total |= flag_value | ||
| if enum_class._boundary_ is not KEEP: | ||
| missed = list(_iter_bits_lsb(multi_bit_total & ~single_bit_total)) | ||
| if missed: | ||
| raise TypeError( | ||
| 'invalid Flag %r -- missing values: %s' | ||
| % (cls, ', '.join((str(i) for i in missed))) | ||
| ) | ||
| enum_class._flag_mask_ = single_bit_total | ||
| # | ||
| # set correct __iter__ | ||
|
|
@@ -688,7 +684,10 @@ def __members__(cls): | |
| return MappingProxyType(cls._member_map_) | ||
|
|
||
| def __repr__(cls): | ||
| return "<enum %r>" % cls.__name__ | ||
| if Flag is not None and issubclass(cls, Flag): | ||
| return "<flag %r>" % cls.__name__ | ||
| else: | ||
| return "<enum %r>" % cls.__name__ | ||
|
|
||
| def __reversed__(cls): | ||
| """ | ||
|
|
@@ -1303,7 +1302,8 @@ def __invert__(self): | |
| else: | ||
| # calculate flags not in this member | ||
| self._inverted_ = self.__class__(self._flag_mask_ ^ self._value_) | ||
| self._inverted_._inverted_ = self | ||
| if isinstance(self._inverted_, self.__class__): | ||
| self._inverted_._inverted_ = self | ||
| return self._inverted_ | ||
|
|
||
|
|
||
|
|
@@ -1561,6 +1561,91 @@ def convert_class(cls): | |
| return enum_class | ||
| return convert_class | ||
|
|
||
| @_simple_enum(StrEnum) | ||
| class EnumCheck: | ||
| """ | ||
| various conditions to check an enumeration for | ||
| """ | ||
| CONTINUOUS = "no skipped integer values" | ||
| NAMED_FLAGS = "multi-flag aliases may not contain unnamed flags" | ||
| UNIQUE = "one name per value" | ||
| CONTINUOUS, NAMED_FLAGS, UNIQUE = EnumCheck | ||
|
|
||
|
|
||
| class verify: | ||
| """ | ||
| Check an enumeration for various constraints. (see EnumCheck) | ||
| """ | ||
| def __init__(self, *checks): | ||
| self.checks = checks | ||
| def __call__(self, enumeration): | ||
| checks = self.checks | ||
| cls_name = enumeration.__name__ | ||
| if Flag is not None and issubclass(enumeration, Flag): | ||
| enum_type = 'flag' | ||
| elif issubclass(enumeration, Enum): | ||
| enum_type = 'enum' | ||
| else: | ||
| raise TypeError("the 'verify' decorator only works with Enum and Flag") | ||
| for check in checks: | ||
| if check is UNIQUE: | ||
| # check for duplicate names | ||
| duplicates = [] | ||
| for name, member in enumeration.__members__.items(): | ||
| if name != member.name: | ||
| duplicates.append((name, member.name)) | ||
| if duplicates: | ||
| alias_details = ', '.join( | ||
| ["%s -> %s" % (alias, name) for (alias, name) in duplicates]) | ||
| raise ValueError('aliases found in %r: %s' % | ||
| (enumeration, alias_details)) | ||
| elif check is CONTINUOUS: | ||
| values = set(e.value for e in enumeration) | ||
| if len(values) < 2: | ||
| continue | ||
| low, high = min(values), max(values) | ||
| missing = [] | ||
| if enum_type == 'flag': | ||
| # check for powers of two | ||
| for i in range(_high_bit(low)+1, _high_bit(high)): | ||
| if 2**i not in values: | ||
| missing.append(2**i) | ||
| elif enum_type == 'enum': | ||
| # check for powers of one | ||
| for i in range(low+1, high): | ||
| if i not in values: | ||
| missing.append(i) | ||
| else: | ||
| raise Exception('verify: unknown type %r' % enum_type) | ||
| if missing: | ||
| raise ValueError('invalid %s %r: missing values %s' % ( | ||
| enum_type, cls_name, ', '.join((str(m) for m in missing))) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it make sense to limit missing to
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes it would.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can make a PR on the same issue, if that's okay?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [I merged it after your comment, as it fixes a regression; we can still make your change if needed.] Having thought about it a little more, I'm not sure we need that: For any static
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Both seem pretty unlikely so I'm not sure it's a needed fix, just wanted to point it out.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good points. I changed the error message to list the problem aliases and the combined value of missing flags, then limited that to 256 characters. Thank you for your help! |
||
| ) | ||
| elif check is NAMED_FLAGS: | ||
| # examine each alias and check for unnamed flags | ||
| member_names = enumeration._member_names_ | ||
| member_values = [m.value for m in enumeration] | ||
| missing = [] | ||
| for name, alias in enumeration._member_map_.items(): | ||
| if name in member_names: | ||
| # not an alias | ||
| continue | ||
| values = list(_iter_bits_lsb(alias.value)) | ||
| missed = [v for v in values if v not in member_values] | ||
| if missed: | ||
| plural = ('', 's')[len(missed) > 1] | ||
| a = ('a ', '')[len(missed) > 1] | ||
| missing.append('%r is missing %snamed flag%s for value%s %s' % ( | ||
| name, a, plural, plural, | ||
| ', '.join(str(v) for v in missed) | ||
| )) | ||
| if missing: | ||
| raise ValueError( | ||
| 'invalid Flag %r: %s' | ||
| % (cls_name, '; '.join(missing)) | ||
| ) | ||
| return enumeration | ||
|
|
||
| def _test_simple_enum(checked_enum, simple_enum): | ||
| """ | ||
| A function that can be used to test an enum created with :func:`_simple_enum` | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any guidance on when to use
@uniquevs@verify(UNIQUE)?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only difference between the two is
uniqueraisesValueErrorandverifyraisesTypeError. I don't seeuniquegoing away, so it's pretty much personal preference.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed that so both raise
ValueError. I addedUNIQUEtoverify()for two reasons:unique()could eventually go away (not that it will any time soon)