-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Mark more things as literals #20604
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?
Mark more things as literals #20604
Conversation
This comment has been minimized.
This comment has been minimized.
for more information, see https://pre-commit.ci
This comment has been minimized.
This comment has been minimized.
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
JukkaL
left a comment
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.
Does this have any behavior change? If yes, consider adding a test case.
| Key: _TypeAlias = tuple[Any, ...] | ||
| LITERAL_NAMES = frozenset( | ||
| ("builtins.True", "builtins.False", "builtins.None", "builtins.NotImplemented") | ||
| ) |
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.
As micro-optimization, can you make this Final? This is potentially perfomance-critical, and Final will make lookups faster when using mypyc.
| if e.fullname in LITERAL_NAMES: | ||
| return LITERAL_YES | ||
| elif isinstance(e.node, TypeAlias) and not e.node.python_3_12_type_alias: | ||
| if is_named_instance(e.node.target, LITERAL_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.
Can a type alias refer to anything but None out of the options here? If this is case, maybe avoid LITERAL_NAMES.
Followup to #20492. This is basically just refactoring based on what seems more correct.