-
-
Notifications
You must be signed in to change notification settings - Fork 34.7k
gh-149828: make Protocol compatible with Enum instantiation #149830
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
base: main
Are you sure you want to change the base?
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 |
|---|---|---|
|
|
@@ -2478,6 +2478,26 @@ class SomeEnum(Enum): | |
| globals()['T'] = T | ||
| test_pickle_dump_load(self.assertIs, SomeEnum.first) | ||
|
|
||
| def test_protocol_mixin_as_enum_member_value(self): | ||
| class Example(typing.Protocol): | ||
| def method(self) -> None: ... | ||
|
|
||
| class Impl(Example): | ||
| def method(self) -> None: ... | ||
|
|
||
| class CompatEnumType(type(typing.Protocol), EnumType): ... | ||
|
|
||
| class ProtoEnum(Example, Enum, metaclass=CompatEnumType): | ||
| __qualname__ = 'ProtoEnum' # needed for pickle protocol 4 | ||
| impl = Impl() | ||
|
randolf-scholz marked this conversation as resolved.
|
||
|
|
||
| self.assertIsInstance(ProtoEnum.impl.value, Impl) | ||
| globals()['Example'] = Example | ||
| globals()['Impl'] = Impl | ||
| globals()['ProtoEnum'] = ProtoEnum | ||
|
Comment on lines
+2495
to
+2497
Member
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 also feel like we shouldn't be mutating the global environment in a test like this. If these classes need to be global, can't we just define them in the global scope? I was honestly surprised the refleak buildbots didn't complain about this
Member
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. They don't because we run a warmup round first; refleak buildbots only complain if you keep leaking on every test.
Member
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. ah makes sense |
||
| ProtoEnum.__reduce_ex__ = enum.pickle_by_enum_name | ||
| test_pickle_dump_load(self.assertIs, ProtoEnum.impl) | ||
|
|
||
| def test_duplicate_values_give_unique_enum_items(self): | ||
| class AutoNumber(Enum): | ||
| first = () | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1882,7 +1882,11 @@ def _get_protocol_attrs(cls): | |
| def _no_init_or_replace_init(self, *args, **kwargs): | ||
| cls = type(self) | ||
|
|
||
| if cls._is_protocol: | ||
| # Determine if this is a protocol or a concrete subclass. | ||
| # Note: not using cls._is_protocol here, | ||
| # since this may be called before __init_subclass__ is resolved. | ||
| # for example when subclassing enum.Enum. | ||
| if any(b is Protocol for b in cls.__bases__): | ||
|
Member
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. Could this break when inheriting from |
||
| raise TypeError('Protocols cannot be instantiated') | ||
|
|
||
| # Already using a custom `__init__`. No need to calculate correct | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Fixed an issue that prevented creation of an :class:`enum.Enum` that inherits from a subclass :class:`typing.Protocol`. |
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 change you made was to typing.py. Can you add a test case to test_typing that demonstrates the issue without relying on enum itself?