-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Make Counter generic over the value #11632
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: main
Are you sure you want to change the base?
Changes from 9 commits
e7e430a
fc7af55
ab7f64c
44ed688
923d8e2
f460e39
424d3c0
e336926
2f5105b
dcf86cf
caa5b14
e10c23c
b107d20
4723e02
39aa741
f9a14ba
3fe47e0
7bcdcff
a15d407
2b1a6a0
6cc18d2
4e4fdc6
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 |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| from __future__ import annotations | ||
|
|
||
| from collections import Counter | ||
| from typing_extensions import assert_type | ||
|
|
||
|
|
||
| class Foo: ... | ||
|
|
||
|
|
||
| # Test the constructor | ||
| # assert_type(Counter(), Counter[Never, int]) # pyright derives "Unknown" instead of "Never" | ||
| assert_type(Counter(foo=42.2), "Counter[str, float]") | ||
| assert_type(Counter({42: "bar"}), "Counter[int, str]") | ||
| assert_type(Counter([1, 2, 3]), "Counter[int, int]") | ||
|
|
||
| int_c: Counter[str] = Counter() | ||
| assert_type(int_c, "Counter[str, int]") | ||
| assert_type(int_c["a"], int) | ||
| int_c["a"] = 1 | ||
| int_c["a"] += 3 | ||
| int_c["a"] += 3.5 # type: ignore | ||
|
|
||
| float_c = Counter(foo=42.2) | ||
| assert_type(float_c, "Counter[str, float]") | ||
| assert_type(float_c["a"], float) | ||
| float_c["a"] = 1.0 | ||
| float_c["a"] += 3.0 | ||
| float_c["a"] += 42 | ||
| float_c["a"] += "42" # type: ignore | ||
|
|
||
| custom_c: Counter[str, Foo] = Counter() | ||
| assert_type(custom_c, "Counter[str, Foo]") | ||
| assert_type(custom_c["a"], Foo) | ||
|
Member
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. At runtime this is actually an int though. I wonder if we need to make all these methods return
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. This line should probably not be accepted, as
Member
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. Doesn't this sort of problem apply to any Counter with a non-int value type, though? This seems like a fundamental problem with this PR.
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.
I wonder whether type checkers support |
||
| custom_c["a"] = Foo() | ||
| custom_c["a"] += Foo() # type: ignore | ||
| # pyright incorrectly accepts the next line | ||
| # custom_c["a"] += 42 # xtype: ignore | ||
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.
For what it's worth I think that's better than
Never. We could test the value type with something like: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.
Unfortunately, this makes pyright unhappy as well.
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.
assert_type(Counter(), "Counter[Any, int]")works.