stdlib: Add several missing comparison methods - #7202
JelleZijlstra merged 4 commits into
Conversation
Sorry, something went wrong.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| def __ior__(self: Self, other: Counter[_T]) -> Self: ... # type: ignore[override] | ||
| if sys.version_info >= (3, 10): | ||
| def total(self) -> int: ... | ||
| def __le__(self, other: Counter[object]) -> bool: ... |
There was a problem hiding this comment.
I think we should use Counter[Any] here. It doesn't work with all Counters, only with Counters of a type that's comparable to our type. That's something we can't express in the current type system, so we should use Any.
Sorry, something went wrong.
There was a problem hiding this comment.
>>> from collections import Counter
>>> Counter('abcde') < Counter((1, 2, 3))
FalseWhat am I missing? :)
Sorry, something went wrong.
There was a problem hiding this comment.
Ah right, the type parameter is the keys, but it compares the values, which are always ints.
This does make for some fun behavior:
In [16]: Counter((1, 2, 3)) < Counter('abcde')
Out[16]: False
In [17]: Counter('abcde') < Counter((1, 2, 3))
Out[17]: False
Sorry, something went wrong.
There was a problem hiding this comment.
Counters are sort of halfway between dicts and sets, and the comparison methods come from the set-like side of their personality, so it does sort of make sense if you squint at it from the right angle.
Sorry, something went wrong.
|
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
Sorry, something went wrong.
| filename: str | ||
| lineno: int | ||
| def __init__(self, frame: _FrameTupleT) -> None: ... | ||
| def __lt__(self, other: Frame) -> bool: ... |
There was a problem hiding this comment.
Confirmed that __lt__ for some reason does not have the NotImplemented trick.
Sorry, something went wrong.
There was a problem hiding this comment.
It's because it's functools.total_ordering that uses the NotImplemented trick, and __lt__ is the only method that's actually defined on the class (all the other methods are autogenerated using @functools.total_ordering).
Sorry, something went wrong.
There was a problem hiding this comment.
This recent change is why I used the sys.version_info guards, FWIW: python/cpython#30818
Sorry, something went wrong.
No description provided.