8000 stdlib: Add several missing comparison methods by AlexWaygood · Pull Request #7202 · python/typeshed · GitHub
[go: up one dir, main page]

Skip to content

stdlib: Add several missing comparison methods #7202

New issue 8000

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

Merged
merged 4 commits into from
Feb 14, 2022

Conversation

AlexWaygood
Copy link
Member

No description provided.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@@ -266,6 +270,10 @@ class Counter(dict[_T, int], Generic[_T]):
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: ...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

>>> from collections import Counter
>>> Counter('abcde') < Counter((1, 2, 3))
False

What am I missing? :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author
@AlexWaygood AlexWaygood Feb 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@@ -41,6 +41,15 @@ class Frame:
filename: str
lineno: int
def __init__(self, frame: _FrameTupleT) -> None: ...
def __lt__(self, other: Frame) -> bool: ...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed that __lt__ for some reason does not have the NotImplemented trick.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Member Author
@AlexWaygood AlexWaygood Feb 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This recent change is why I used the sys.version_info guards, FWIW: python/cpython#30818

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0