-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Add missing _add_value_alias_ method for Python 3.13 enum #14411
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?
Conversation
Fixes python#14408 by adding the _add_value_alias_ method to the Enum class with proper Python 3.13 version guard. Also adds comprehensive test cases for the MultiValueEnum pattern. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
cb06731
to
693d71d
Compare
693d71d
to
1e82dac
Compare
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
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.
Thanks, remarks below. We should also add overridden variants of this method to StrEnum
and IntEnum
, similar to what we do for other attributes/methods where value
is used.
Also, for future reference: Please don't use AI PR summaries. Those use a lot of words for something very simple, making it considerably harder to understand the issue and PR. In this case, a simple link back to the original issued would have sufficed.
|
||
|
||
if sys.version_info >= (3, 13): | ||
|
||
class MultiValueEnum(enum.Enum): | ||
def __new__(cls, value: object, *values: Any) -> "MultiValueEnum": | ||
self = object.__new__(cls) | ||
self._value_ = value | ||
for v in values: | ||
self._add_value_alias_(v) | ||
return self | ||
|
||
class DType(MultiValueEnum): | ||
float32 = "f", 8 | ||
double64 = "d", 9 | ||
|
||
# Test type inference for primary values | ||
assert_type(DType("f"), DType) | ||
assert_type(DType("d"), DType) | ||
|
||
# Test type inference for alias values | ||
assert_type(DType(8), DType) | ||
assert_type(DType(9), DType) | ||
|
||
# Test that the enum members have the correct literal types | ||
assert_type(DType.float32, Literal[DType.float32]) | ||
assert_type(DType.double64, Literal[DType.double64]) |
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.
These tests don't seem to test anything relevant – at least not as it relates to type annotations. We generally don't run our test cases, we just type check them, so any logic is meaningless. This is also seems to test something called a MultiValueEnum
, which isn't a concept in Python's stdlib.
@@ -219,6 +219,8 @@ class Enum(metaclass=EnumMeta): | |||
if sys.version_info >= (3, 12) and sys.version_info < (3, 14): | |||
@classmethod | |||
def __signature__(cls) -> str: ... | |||
if sys.version_info >= (3, 13): | |||
def _add_value_alias_(self, value: Any) -> None: ... |
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.
We nowadays require explanatory comments for all non-obvious Any
s. In this case, maybe the following comment makes sense?
def _add_value_alias_(self, value: Any) -> None: ... | |
# value must have the same type as other enum members | |
def _add_value_alias_(self, value: Any) -> None: ... |
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.
I would like to note that the _add_alias_
method was added in the same version, and it would be possible to add them together
Thank you for your comments. Give me a day or two to address them and correct this PR. I will add the requisite comments, remove the irrelevant tests, and check the additional special enum types. Also, my apologies for the verbose PR summary. I was really just letting Claude run rampant and see what he did / what he came up with. I'll keep that in mind for the future. |
Fixes #14408
Summary
_add_value_alias_
method toEnum
class for Python 3.13Fixes #14408 by resolving the type checker error when using
_add_value_alias_
method in Python 3.13 enum classes.Changes Made
_add_value_alias_(self, value: Any) -> None
method toEnum
class instdlib/enum.pyi
if sys.version_info >= (3, 13):
)stdlib/@tests/test_cases/check_enum.py
including:Test Plan
Example Usage
🤖 Generated with Claude Code