-
-
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
Conversation
This comment has been minimized.
This comment has been minimized.
693d71d
to
1e82dac
Compare
This comment has been minimized.
This comment has been minimized.
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.
Hmm... Looking at your suggestion... is this true? I have implemented a str enum with int and other aliases...
from enum import Enum, StrEnum
class EOTFType(str, Enum):
RESERVED = ("RESERVED", 0)
SDR = ("SDR", 1)
PQ = ("PQ", 2)
HLG = ("HLG", 3)
def __new__(cls, value: str, int_val: int):
self = str.__new__(cls, value)
self._value_ = value
self._add_value_alias_(int_val) # type: ignore Added in python 3.13
return self
def __init__(
self,
_: str,
int_value: int,
):
self.int_value = int_value
def __str__(self) -> str:
"""
Return a formatted string representation of the EOTF type.
Returns
-------
str
Format: "value=name" (e.g., "2=PQ")
"""
return f'{self.value}="{self.value}"={self.int_value}'
class StrIntEnum(StrEnum):
RED = "red"
GREEN = "green"
def main():
direct = EOTFType.PQ
print(direct)
parsed_int = EOTFType(2)
print(parsed_int)
parsed_str = EOTFType("PQ")
print(parsed_str)
print(f'parsed_str == "PQ": {parsed_str == "PQ"}')
print(f'parsed_str == "PQ": {parsed_str == 2}')
print(f'parsed_str.int_value == "PQ": {parsed_str.int_value == 2}')
StrIntEnum.GREEN._add_value_alias_(4)
print(StrIntEnum(4))
if __name__ == "__main__":
main()
Is Any really the correct annotation here or is object
better... as above?
As for the special enums (StrEnum, IntEnum)... It seems the alias can still be "any".
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 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 <[email protected]>
ebc2804
to
c205ac0
Compare
This comment has been minimized.
This comment has been minimized.
Bringing this in-line with https://github.com/python/cpython/blob/3.13/Lib/enum.py#L1220-L1246 It seems like the docs page is missing the function signature. This is my first http://github.com/python contribution, so I may be a bit lost about what is correct or incorrect. Python Docs: EnumType._add_value_alias I believe that IntEnum and StrEnum simply inherit the Enum behavior and do not impose additional type restrictions. Therefore, they do not need specific overrides for type checking. |
8776fc7
to
04b5750
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!
Fixes #14408