Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork1.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 ourterms of service andprivacy 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?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Fixespython#14408 by adding the _add_value_alias_ method to the Enum classwith proper Python 3.13 version guard. Also adds comprehensive testcases 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 to693d71dCompare693d71d to1e82dacCompareAccording tomypy_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 toStrEnum andIntEnum, similar to what we do for other attributes/methods wherevalue 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 aMultiValueEnum, 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-obviousAnys. 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. |
Uh oh!
There was an error while loading.Please reload this page.
Fixes#14408
Summary
_add_value_alias_method toEnumclass 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) -> Nonemethod toEnumclass instdlib/enum.pyiif sys.version_info >= (3, 13):)stdlib/@tests/test_cases/check_enum.pyincluding:Test Plan
Example Usage
🤖 Generated withClaude Code