-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Fix enum attributes are not members #17207
base: master
Are you sure you want to change the base?
Fix enum attributes are not members #17207
Conversation
f319685
to
ba435ac
Compare
This comment has been minimized.
This comment has been minimized.
isinstance(sym.node, mypy.nodes.Var) | ||
and name not in ENUM_REMOVED_PROPS | ||
and not name.startswith("__") | ||
and sym.node.has_explicit_value |
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.
This last condition changes the behavior of mypy which currently considers unassigned but annotated attributes as members of the enum.
I would say the new behavior is better but it must be noted in the PR title or description that this is a breaking change that must be considered on its own.
Looking at the mypy primer output, it looks like members of enums constructed using the enum call syntax do not have the has_explicit_value
set which breaks type narrowing. I suggest changing the variable here to also set this flag perhaps with a comment explaining the reason -- something like "E = Enum("E", "A")
is equivalent to class E(Enum): A = auto()
"
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 for noticing that, I've not really used them in that form, but hopefully the comment is not too wordy. I only glanced at the code that you pointed to, but I was cross referencing https://github.com/python/cpython/blob/7528b84e947f727734bd802356e380673553387d/Lib/enum.py#L828-L839 and how much of this is implemented by mypy? 🤔
This comment has been minimized.
This comment has been minimized.
It looks like this is actually related to the ongoing discussion python/typing-council#11 |
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.
Thank you @terencehonles.
The typing spec change has been accepted and this PR looks good to me.
Regarding the mypy primer diff, it is related to the linked typing spec change that requires assignment for enum attributes for them to be considered members of the enumeration and the fact that tanjun vendors an old copy of the stdlib inspect module and stub that relies on the old behavior. So the new errors are expected.
This adds on to the change in python#17182 and fixes enum attributes being used as members.
5383c49
to
dfcc0c4
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@hamdanal sorry for the delay getting back to this, I went on vacation and then coming back was pretty busy. Work's on break for the week so I figured I could pick this up to keep it from getting delayed any further. I was having trouble forcing mypy to not use the explicitly provided annotation for an enum member and I will probably have to leave it at this. I left a comment https://github.com/python/mypy/pull/17207/files#diff-8b48b1eca587106e3806bfa9e14b7f7f344e117b203ffc716ce6fef7f2e68fe6R1610 about the test that was added for #11971. The code in question will need to ignore this new error, but as mypy better supports enums the code will need to change as it seems like it's abusing enums a bit. |
This comment has been minimized.
This comment has been minimized.
This change removes the type attributes on WebsocketNotification which will become errors as of python/mypy#17207 see: https://typing.readthedocs.io/en/latest/spec/enums.html#defining-members:~:text=Members%20defined%20within%20an%20enum%20class%20should%20not%20include%20explicit%20type%20annotations.
@hamdanal / @JelleZijlstra friendly ping if you have a chance to look over the PR or suggestions on how to fix ^^^ |
Diff from mypy_primer, showing the effect of this PR on open source code: Tanjun (https://github.com/FasterSpeeding/Tanjun)
+ tanjun/_internal/__init__.py:189: error: Statement is unreachable [unreachable]
+ tanjun/annotations.py:2646: error: Statement is unreachable [unreachable]
pytest (https://github.com/pytest-dev/pytest)
+ src/_pytest/scope.py:36: error: Type annotations are not allowed for enum members [misc]
+ src/_pytest/scope.py:37: error: Type annotations are not allowed for enum members [misc]
+ src/_pytest/scope.py:38: error: Type annotations are not allowed for enum members [misc]
+ src/_pytest/scope.py:39: error: Type annotations are not allowed for enum members [misc]
+ src/_pytest/scope.py:40: error: Type annotations are not allowed for enum members [misc]
jax (https://github.com/google/jax)
+ jaxlib/cpu/_lapack/eig.pyi:20: error: Type annotations are not allowed for enum members [misc]
+ jaxlib/cpu/_lapack/eig.pyi:21: error: Type annotations are not allowed for enum members [misc]
|
This adds on to the change in #17182 and fixes enum attributes being used as members.
@hamdanal / @hauntsaninja I noticed there was the function
get_enum_values
in mypy types and instead of adding more to the case intry_expanding_sum_type_to_union
it seemed like it might make sense to move the changes from #17182 there. I don't think the other code touched in that PR can useget_enum_values
.fixes: #16730