-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Make IntEnum/StrEnum values passable to functions expecting literal ints or strs #19617
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: master
Are you sure you want to change the base?
Conversation
According to mypy_primer, this change doesn't affect type check results on a corpus of 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.
Great, thanks! Left a couple of stylistic comments, LG overall. We definitely should consider enum literals subtypes of their corresponding value literals.
# This handles IntEnum, StrEnum, and custom (int, Enum) or (str, Enum) subclasses | ||
if ( | ||
left.is_enum_literal() | ||
and isinstance(left.value, str) # Enum literal values are member names |
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.
Better make this assert
inside the if body: we want a hard failure if enum literal somehow ended up with a non-string value
if isinstance(enum_type, Instance) and enum_type.last_known_value is not None: | ||
# enum_type.last_known_value is the actual value for IntEnum/StrEnum | ||
# members | ||
if enum_type.last_known_value.value == self.right.value: | ||
return True |
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.
if isinstance(enum_type, Instance) and enum_type.last_known_value is not None: | |
# enum_type.last_known_value is the actual value for IntEnum/StrEnum | |
# members | |
if enum_type.last_known_value.value == self.right.value: | |
return True | |
if isinstance(enum_type, Instance) and enum_type.last_known_value == self.right.value: | |
# enum_type.last_known_value is the actual value for IntEnum/StrEnum | |
# members | |
return True |
Let's collapse the ladder?
Ough, Eric raised a good point in the linked ticket. This is indeed unsafe, and probably |
Fixes #19616.
Some design considerations:
MyIntEnum.ONE
where aLiteral[1]
is expected; this is becauseisinstance(MyIntEnum.ONE, int) == True
.1
would be passed where aMyIntEnum
argument is expected: this is because two distinct enums that are semantically imcompatible might share numerical values. We want to emit errors if the user misuses an enum.