-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Allow literals as kwargs dict keys #20416
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -334,7 +334,7 @@ def f(**kwargs: 'A') -> None: pass | |
| b: Mapping | ||
| d: Mapping[A, A] | ||
| m: Mapping[str, A] | ||
| f(**d) # E: Keywords must be strings | ||
| f(**d) # E: Argument after ** must have string keys | ||
| f(**m) | ||
| f(**b) | ||
| class A: pass | ||
|
|
@@ -354,7 +354,7 @@ from typing import Dict, Any, Optional | |
| class A: pass | ||
| def f(**kwargs: 'A') -> None: pass | ||
| d = {} # type: Dict[A, A] | ||
| f(**d) # E: Keywords must be strings | ||
| f(**d) # E: Argument after ** must have string keys | ||
| f(**A()) # E: Argument after ** must be a mapping, not "A" | ||
| kwargs: Optional[Any] | ||
| f(**kwargs) # E: Argument after ** must be a mapping, not "Any | None" | ||
|
|
@@ -449,7 +449,7 @@ f(b) # E: Argument 1 to "f" has incompatible type "dict[str, str]"; expected "in | |
| f(**b) # E: Argument 1 to "f" has incompatible type "**dict[str, str]"; expected "int" | ||
|
|
||
| c = {0: 0} | ||
| f(**c) # E: Keywords must be strings | ||
| f(**c) # E: Argument after ** must have string keys | ||
| [builtins fixtures/dict.pyi] | ||
|
|
||
| [case testCallStar2WithStar] | ||
|
|
@@ -567,3 +567,13 @@ main:38: error: Argument after ** must be a mapping, not "C[str, float]" | |
| main:39: error: Argument after ** must be a mapping, not "D" | ||
| main:41: error: Argument 1 to "foo" has incompatible type "**dict[str, str]"; expected "float" | ||
| [builtins fixtures/dict.pyi] | ||
|
|
||
| [case testLiteralKwargs] | ||
| from typing import Any, Literal | ||
| kw: dict[Literal["a", "b"], Any] | ||
| def func(a, b): ... | ||
| func(**kw) | ||
|
|
||
| badkw: dict[Literal["one", 1], Any] | ||
| func(**badkw) # E: Argument after ** must have string keys | ||
| [builtins fixtures/dict.pyi] | ||
|
Comment on lines
+571
to
+579
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This modified test produces false-positive for # [case testLiteralKwargs]
from typing import Literal
def func(a: int, b: int) -> None: ...
class A: pass
good_kw: dict[Literal["a", "b"], int]
bad_kw: dict[Literal["one", 1], int]
func(**good_kw)
func(**bad_kw) # E: Argument after ** must have string keys
# [builtins fixtures/dict.pyi]
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, this PR does handle correctly handle that test case. The patch is ad hoc though, so e.g. it would fail if you changed it to Mapping
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right, I think tested with the wrong commit
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did make the proposed change into a helper function: #20419 |
||
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.
Imo this whole approach is technically incorrect.
SupportsKeysAndGetItemis invariant in key type, so prescribingstrwill cause errors if a dict with literal keys is given