-
-
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
Allow TypedDict key with literal type during construction #7645
Conversation
Refactored an operation away from `mypy.plugins.common` so that it is more cleanly reusable. Also kept an alias in the original location to avoid breaking existing plugins. Fixes #7644.
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, LGTM! Just one comment about tests.
test-data/unit/check-typeddict.test
Outdated
num2: Literal['num'] | ||
v = {num2: 2} | ||
bad2: Literal['bad'] | ||
v = {bad: 2} # E: Expected TypedDict key to be string literal |
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 suppose this should be bad2
, otherwise bad2
would be unused in this test.
for lit in get_proper_types(possible_literals): | ||
if isinstance(lit, LiteralType) and lit.fallback.type.fullname() == 'builtins.str': | ||
val = lit.value | ||
assert isinstance(val, str) |
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.
It's not a review comment, just my curiosity: why is this needed?
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.
Mypy thinks that the value could be an integer, for example, since it doesn't understand the fullname()
check above.
Also refactored an operation away from
mypy.plugins.common
so that itis more cleanly reusable. Also kept an alias in the original location
to avoid breaking existing plugins.
Fixes #7644.