-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Do not allow type[]
to contain Literal
types
#18276
Conversation
for more information, see https://pre-commit.ci
@@ -81,14 +82,14 @@ def is_bad_type_type_item(item: Type) -> bool: | |||
Such types are explicitly prohibited by PEP 484. Also, they cause problems | |||
with recursive types like T = Type[T], because internal representation of | |||
TypeType item is normalized (i.e. always a proper type). | |||
|
|||
Also forbids `Type[Literal[...]]`, because typing spec does not allow it. |
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 think it might be better to frame this in terms of what is allowed. Here are some other things mypy allows that I don't think make sense: https://mypy-play.net/?mypy=latest&python=3.12&gist=aa900cb6c86fd55901b21ee182aa35ec
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 suspected that this will be brought up :)
I would prefer to keep this PR focused on just one thing. Because, for example, I only agree with the first example: type[TypeGuard[int]]
, but it is a completely different thing to fix.
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 think that we can clarify this in https://github.com/python/typing/tree/main/docs/spec
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 is already specified in the grammar at https://typing.readthedocs.io/en/latest/spec/annotations.html#type-and-annotation-expressions . (Though I noticed we're missing unions there, which should be supported.)
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'm fine with deferring other similar changes to other PRs, though.
This comment has been minimized.
This comment has been minimized.
for more information, see https://pre-commit.ci
Diff from mypy_primer, showing the effect of this PR on open source code: bokeh (https://github.com/bokeh/bokeh)
+ src/bokeh/core/property/enum.py: note: In function "__init__":
+ src/bokeh/core/property/enum.py:64:30: error: type[...] can't contain "Literal[...]" [valid-type]
+ src/bokeh/core/property/enum.py:70:36: error: type[...] can't contain "Literal[...]" [valid-type]
|
Closes #18196