Skip to content
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

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

Open
jorenham opened this issue Aug 20, 2024 · 8 comments · May be fixed by #12573 or #12939
Open

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

jorenham opened this issue Aug 20, 2024 · 8 comments · May be fixed by #12573 or #12939
Labels
stubs: false positive Type checkers report false errors

Comments

@jorenham
Copy link
Contributor

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 (numpy/numpy#26942).

But as was noticed in numpy/numpy#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)
@AlexWaygood AlexWaygood changed the title False negative with builtins.min(T, T) if T.__lt__ doesn't return a builtins.bool False positive with builtins.min(T, T) if T.__lt__ doesn't return a builtins.bool Aug 20, 2024
@AlexWaygood AlexWaygood added the stubs: false positive Type checkers report false errors label Aug 20, 2024
@srittau
Copy link
Collaborator

srittau commented Aug 21, 2024

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

class SupportsDunderLT(Protocol[_T_contra]):
def __lt__(self, other: _T_contra, /) -> bool: ...
class SupportsDunderGT(Protocol[_T_contra]):
def __gt__(self, other: _T_contra, /) -> bool: ...
class SupportsDunderLE(Protocol[_T_contra]):
def __le__(self, other: _T_contra, /) -> bool: ...
class SupportsDunderGE(Protocol[_T_contra]):
def __ge__(self, other: _T_contra, /) -> bool: ...

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
Copy link

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
Copy link
Contributor

This should be solvable by returning SupportsBool where

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

@JelleZijlstra
Copy link
Member

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
Copy link
Contributor

randolf-scholz commented Aug 21, 2024

@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
Copy link
Contributor

randolf-scholz commented Aug 21, 2024

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
Copy link
Contributor

randolf-scholz commented Aug 21, 2024

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

class _SupportsDunderLT(Protocol):
def __lt__(self, other: Any, /) -> Any: ...
class _SupportsDunderGT(Protocol):
def __gt__(self, other: Any, /) -> Any: ...
class _SupportsDunderLE(Protocol):
def __le__(self, other: Any, /) -> Any: ...
class _SupportsDunderGE(Protocol):
def __ge__(self, other: Any, /) -> Any: ...
_SupportsComparison: TypeAlias = _SupportsDunderLE | _SupportsDunderGE | _SupportsDunderGT | _SupportsDunderLT

@randolf-scholz
Copy link
Contributor

randolf-scholz commented Aug 21, 2024

PR #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.

nsoranzo added a commit to nsoranzo/galaxy that referenced this issue Aug 30, 2024
caused by the more precise type annotation of ``numpy.number`` comparison
operators introduced in NumPy 2.1, see:

python/typeshed#12562

Fix the following errors when running test_galaxy_packages on Python >=3.10:

```
galaxy/tool_util/verify/__init__.py:503: error: Value of type variable "SupportsRichComparisonT" of "max" cannot be "floating[Any]"  [type-var]
                iou_list.append(max(cc1_iou_list))
                                ^~~~~~~~~~~~~~~~~
galaxy/tool_util/verify/__init__.py:532: error: Value of type variable "SupportsRichComparisonT" of "min" cannot be "floating[Any]"  [type-var]
        return min(_multiobject_intersection_over_union(mask1, mask2, pin_labels))
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```
nsoranzo added a commit to nsoranzo/galaxy that referenced this issue Aug 30, 2024
caused by the more precise type annotation of ``numpy.number`` comparison
operators introduced in NumPy 2.1, see:

python/typeshed#12562

Fix the following errors when running test_galaxy_packages on Python >=3.10:

```
galaxy/tool_util/verify/__init__.py:503: error: Value of type variable "SupportsRichComparisonT" of "max" cannot be "floating[Any]"  [type-var]
                iou_list.append(max(cc1_iou_list))
                                ^~~~~~~~~~~~~~~~~
galaxy/tool_util/verify/__init__.py:532: error: Value of type variable "SupportsRichComparisonT" of "min" cannot be "floating[Any]"  [type-var]
        return min(_multiobject_intersection_over_union(mask1, mask2, pin_labels))
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```
nsoranzo added a commit to nsoranzo/galaxy that referenced this issue Oct 10, 2024
caused by the more precise type annotation of ``numpy.number`` comparison
operators introduced in NumPy 2.1, see:

python/typeshed#12562

Fix the following errors when running test_galaxy_packages on Python >=3.10:

```
galaxy/tool_util/verify/__init__.py:503: error: Value of type variable "SupportsRichComparisonT" of "max" cannot be "floating[Any]"  [type-var]
                iou_list.append(max(cc1_iou_list))
                                ^~~~~~~~~~~~~~~~~
galaxy/tool_util/verify/__init__.py:532: error: Value of type variable "SupportsRichComparisonT" of "min" cannot be "floating[Any]"  [type-var]
        return min(_multiobject_intersection_over_union(mask1, mask2, pin_labels))
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stubs: false positive Type checkers report false errors
Projects
None yet
6 participants