python / typeshed

Collection of library stubs for Python, with static types
Other
4.3k stars 1.73k forks source link

False positive with `builtins.min(T, T)` if `T.__lt__` doesn't return a `builtins.bool` #12562

Open jorenham opened 3 weeks ago

jorenham commented 3 weeks ago

Since NumPy 2.1 all subtypes of numpy.number implement __lt__ that returns a numpy.bool (which isn't a builtins.bool), matching the runtime behavior (https://github.com/numpy/numpy/pull/26942).

But as was noticed in https://github.com/numpy/numpy/issues/27251, the consequence of this is that builtins.min with e.g. numpy.int32 input is now rejected (tested with the latest mypy and pyright).

So at runtime we have

>>> import numpy as np
>>> np.__version__
'2.1.0'
>>> np.int32(8) < np.int32(9)
np.True_
>>> min(np.int32(8), np.int32(9))
np.int32(8)

But mypy 1.11.1 rejects min(np.int32(8), np.int32(9)) as:

error: No overload variant of "min" matches argument types "signedinteger[_32Bit]", "signedinteger[_32Bit]"  [call-overload]

Similarly, Pyright 1.1.376 reports

error: Argument of type "int32" cannot be assigned to parameter "arg1" of type "SupportsRichComparisonT@min" in function "min"
    Type "int32" is incompatible with type "SupportsRichComparison"
      Type "int32" is incompatible with type "SupportsRichComparison"
        "signedinteger[_32Bit]" is incompatible with protocol "SupportsDunderLT[Any]"
          "__lt__" is an incompatible type
            No overloaded function matches type "(other: _T_contra@SupportsDunderLT, /) -> bool"
        "signedinteger[_32Bit]" is incompatible with protocol "SupportsDunderGT[Any]"
          "__gt__" is an incompatible type
            No overloaded function matches type "(other: _T_contra@SupportsDunderGT, /) -> bool" (reportArgumentType)

and

error: Argument of type "int32" cannot be assigned to parameter "arg2" of type "SupportsRichComparisonT@min" in function "min"
    Type "int32" is incompatible with type "SupportsRichComparison"
      Type "int32" is incompatible with type "SupportsRichComparison"
        "signedinteger[_32Bit]" is incompatible with protocol "SupportsDunderLT[Any]"
          "__lt__" is an incompatible type
            No overloaded function matches type "(other: _T_contra@SupportsDunderLT, /) -> bool"
        "signedinteger[_32Bit]" is incompatible with protocol "SupportsDunderGT[Any]"
          "__gt__" is an incompatible type
            No overloaded function matches type "(other: _T_contra@SupportsDunderGT, /) -> bool" (reportArgumentType)
srittau commented 3 weeks ago

I think this ultimately boils down to the definition of SupportsDunderLT etc:

https://github.com/python/typeshed/blob/86e74163b9f4ddf2b36e8cac05da11d063ea636b/stdlib/_typeshed/__init__.pyi#L87-L97

These expect the dunders to return a real bool. I'm not sure how np.bool is implemented, so this may be hard to fix.

ngoldbaum commented 3 weeks ago

Unfortunately bool isn't subclassable, so np.bool is its own separate thing. It's backed by 0 and 1 8-bit int values.

randolf-scholz commented 3 weeks ago

This should be solvable by returning SupportsBool where

class SupportsBool(Protocol):
    def __bool__(self) -> bool: ...
JelleZijlstra commented 3 weeks ago

That protocol is equivalent to object in practice, so we'd be better off simply using object as the return type in the SupportDunder* protocols.

randolf-scholz commented 3 weeks ago

@JelleZijlstra At runtime yes, but currently the type hints for object do not include __bool__. Hence, it makes a difference at type checking time: Code sample in pyright playground

from typing import Protocol

class SupportsBool(Protocol):
    def __bool__(self) -> bool: ...

x: SupportsBool = object()  # raises reportAssignmentType

I'm not sure why __bool__ is not declared on object, but there is probably some reason for it. EDIT: It's because object does not have __bool__ at runtime.

randolf-scholz commented 3 weeks ago

OK object seems the right thing as the following works at runtime:

class Foo:
    def __lt__(self, other): return self
    def __gt__(self, other): return self
    def __le__(self, other): return self
    def __ge__(self, other): return self

x = Foo()
y = Foo()
assert min(x, y) is y  # ✅
x.__bool__  # ❌ AttributeError
randolf-scholz commented 3 weeks ago

There is also a very similar definition in _operator.pyi, with this chance these maybe can be combined

https://github.com/python/typeshed/blob/8307b3c36264fa65b4c2e416fdf0527f02f63312/stdlib/_operator.pyi#L21-L33

randolf-scholz commented 3 weeks ago

PR https://github.com/python/typeshed/pull/12573 replaces these bool return hints with object. mypy-primer looks good.

However, it makes me wonder whether the bool return hint should be replaced by object in other cases as well, such as SupportsGetItem.__contains__ and similar protocols.