max() works with greater-than or less-than

Currently max() expects __gt__, but this example indicates that it also works if only __lt__ is present:

class Lt:
    def __init__(self, x):
        self.x = x

    def __repr__(self):
        return f"Lt({self.x})"

    def __lt__(self, other):
        return self.x < other.x

class Gt:
    def __init__(self, x):
        self.x = x

    def __repr__(self):
        return f"Gt({self.x})"

    def __gt__(self, other):
        return self.x > other.x

print(max(Lt(2), Lt(5)))  # Lt(5)
print(max(Lt(5), Lt(2)))  # Lt(5)
print(max(Gt(2), Gt(5)))  # Gt(5)
print(max(Gt(5), Gt(2)))  # Gt(5)

I think that we should accept either method.

The signature of max() was changed to expect __gt__ in #6342.

Having __ge__ or __le__ is not sufficient, however.

@srittau

In #6342, I argued that the signature of max should be the following due to the way comparison operators are desugared by the interpreter :

from typing import Any, Callable, Iterable, Protocol, TypeVar, Union, overload

_T = TypeVar("_T")
_T1 = TypeVar("_T1")
_T2 = TypeVar("_T2")

class _SupportsDunderLT(Protocol):
    def __lt__(self, other: Any) -> bool: ...

class _SupportsDunderGT(Protocol):
    def __gt__(self, other: Any) -> bool: ...

SupportsComparisons = Union[_SupportsDunderGT, _SupportsDunderLT]
SupportsComparisonsT = TypeVar('SupportsComparisonsT', bound=SupportsComparisons)

@overload
def max(
    __arg1: SupportsComparisonsT, __arg2: SupportsComparisonsT, *_args: SupportsComparisonsT, key: None = ...
) -> SupportsComparisonsT: ...
@overload
def max(__arg1: _T, __arg2: _T, *_args: _T, key: Callable[[_T], SupportsComparisons]) -> _T: ...
@overload
def max(__iterable: Iterable[SupportsComparisonsT], *, key: None = ...) -> SupportsComparisonsT: ...
@overload
def max(__iterable: Iterable[_T], *, key: Callable[[_T], SupportsComparisons]) -> _T: ...
@overload
def max(__iterable: Iterable[SupportsComparisonsT], *, key: None = ..., default: _T) -> SupportsComparisonsT| _T: ...
@overload
def max(__iterable: Iterable[_T1], *, key: Callable[[_T1], SupportsComparisons], default: _T2) -> _T1 | _T2: ...

If we make this change, we should probably also update similar functions such as min() and sorted(), which have similar issues. But, I know @srittau disagrees with me 🙂

Related: the mypy issue I opened here, following the conversation in #6342.

Ah, I missed the discussion in the original PR which covers this in detail.

If we'd support this in type checkers, this could result in false negatives if user code tries to call __lt__ or __gt__:

def f(x: SupportsDunderGt, other: Any) -> bool:
    return x.__gt__(other)  # Doesn't do the right thing if __gt__ is missing

f(Lt(5), Lt(6))

The main issue I have is that this would add "ad-hoc" complexity at the type system level, which seems unnecessary in this case. We could easily support this use case in typeshed once, and this would work as expected with every type checker, and without false negatives as far as I can see. If we expect type checkers to support some sort of __lt__/__gt__ equivalence, the implementation is likely to be more difficult to maintain, and it needs to be duplicated for every type checker, including current ones (at least mypy) and any possible future ones. And if some type checker doesn't implement the special case, this will result in false positives for the users of the type checker. It's also possible that different type checkers would implement the rules differently, since this type checking behavior isn't explicitly defined in any PEP, including PEP 544.

Also, I don't think that semantics of PyObject_RichCompareBool are relevant here (this was mentioned in #6342). The protocol class only says that __lt__ or __gt__ is present, and the protocol doesn't talk anything about what is done with __lt__ or __gt__ by the callee. Another function with a signature similar to max() might require __gt__ to be present, and wouldn't use PyObject_RichCompareBool or the equivalent at all.

For these reasons I'd prefer this to be handled in typeshed.

Would a custom "RichComparison" alias make sense here?

If we'd use the definitions from #6575 (comment) above, I think this would make sense.

Maybe something like this?

SupportsRichComparison = Union[_SupportsDunderGT, _SupportsDunderLT]
SupportsRichComparisonT = TypeVar('SupportsRichComparisonT', bound=SupportsRichComparison)

I'd be very interested in submitting a PR along these lines, if we have consensus?

@AlexWaygood

@AlexWaygood I think that we have enough thumbs ups to move forward.

Add `SupportsRichComparison` type to `_typeshed` (#6583)

~~We need to reopen this, unfortunately~~. (EDIT: We don't, see below.) There was a logical error in my PR #6583, in that the following two types are not equivalent:

Callable[[_T], SupportsDunderLT | SupportsDunderGT]

is not the same as

Callable[[_T], SupportsDunderLT] | Callable[[_T], SupportsDunderGT]

The former is incorrect, and is what is currently in the stub in e.g. overloads 2, 4 & 6 of max. The latter is what should be used in these situations.

I think we need another alias in _typeshed — what should it be called? CallableReturningComparable? RichComparisonCallable?

Example

In _typeshed we could have

from typing import Callable, TypeVar, Union

# SupportsDunderLT & SupportsDunderGT still defined as they are currently
_T = TypeVar("_T")
CallableReturningComparable = Union[Callable[[_T], SupportsDunderLT], Callable[[_T], SupportsDunderGT]]

And then max could be rewritten as:

from _typeshed import CallableReturningComparable, SupportsRichComparisonT
from typing import Iterable, TypeVar, overload

_T = TypeVar("_T")

@overload
def max(
    __arg1: SupportsRichComparisonT, __arg2: SupportsRichComparisonT, *_args: SupportsRichComparisonT, key: None = ...
) -> SupportsRichComparisonT: ...
@overload
def max(__arg1: _T, __arg2: _T, *_args: _T, key: CallableReturningComparable[_T]) -> _T: ...
@overload
def max(__iterable: Iterable[SupportsRichComparisonT], *, key: None = ...) -> SupportsRichComparisonT: ...
@overload
def max(__iterable: Iterable[_T], *, key: CallableReturningComparable[_T]) -> _T: ...
@overload
def max(__iterable: Iterable[SupportsRichComparisonT], *, key: None = ..., default: _T) -> SupportsRichComparisonT | _T: ...
@overload
def max(__iterable: Iterable[_T1], *, key: CallableReturningComparable[_T], default: _T2) -> _T1 | _T2: ...

Have you seen issues caused by the new type? I believe that the union return type is fine, since the return type can vary covariantly (Callable[..., X] is valid if Callable[..., X | Y] is expected).

@JukkaL

@JukkaL you're right, please ignore me — it seems to work fine.

Is the following expected? Or is there an issue with either typeshed or the type checker?

def foo(a: SupportsRichComparisonT, b: SupportsRichComparisonT):
    return a < b

main.py:16: error: Unsupported left operand type for < (some union)  [operator]
Found 1 error in 1 file (checked 1 source file)

https://mypy-play.net/?mypy=master&python=3.11&gist=2dcc0cc29ac7d0ee49e87b14a327b301

@Kache

@Kache please open a new issue rather than commenting on one that's been closed for two years :)

Sorry, wasn't sure if it was an actual issue worth posting about.

Navigation Menu

Repository navigation

max() works with greater-than or less-than #6575

Example

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

max() works with greater-than or less-than #6575

Description

Activity

JukkaL commented on Dec 13, 2021

AlexWaygood commented on Dec 13, 2021

JukkaL commented on Dec 13, 2021

srittau commented on Dec 13, 2021

JukkaL commented on Dec 13, 2021

AlexWaygood commented on Dec 13, 2021

JukkaL commented on Dec 14, 2021

AlexWaygood commented on Dec 17, 2021

Example

AlexWaygood commented on Dec 17, 2021

JukkaL commented on Dec 17, 2021

AlexWaygood commented on Dec 17, 2021

Kache commented on Oct 13, 2023

AlexWaygood commented on Oct 13, 2023

Kache commented on Oct 13, 2023

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions