mypy icon indicating copy to clipboard operation
mypy copied to clipboard
verified publisher icon python

Patch `check_op_reversible` to support `tuple` subclasses.

Open randolf-scholz opened this issue 8 months ago • 8 comments

Fixes #19006

  • Added unit test TypeCheckSuite::check-expressions.test::testReverseBinaryOperator4
  • Added fixture tuple-typeshed.pyi that duplicates the tuple definition from typeshed.

This patch drops the is_subtype(right_type, left_type) check in favor of the weaker covers_at_runtime(right_type, left_type).

Open Issues

  • [ ] Some tests were still failing because isinstance(left_type, Instance) fails when left_type is tuple[()]. As a workaround, I changed the unconditional check

    and isinstance(left_type, Instance)
and isinstance(right_type, Instance)
and not (
    left_type.type.alt_promote is not None
    and left_type.type.alt_promote.type is right_type.type
)
and lookup_definer(left_type, op_name) != lookup_definer(right_type, rev_op_name)

    into the conditional check

    # Checking (A implies B) using the logically equivalent (not A or B), where
#    A: left and right are both `Instance` objects
#    B: right's __rop__ method is different from left's __op__ method
not (isinstance(left_type, Instance) and isinstance(right_type, Instance))
or (
    lookup_definer(left_type, op_name) != lookup_definer(right_type, rev_op_name)
    and (
        left_type.type.alt_promote is None
        or left_type.type.alt_promote.type is not right_type.type
    )
)

    But I am not sure this is the correct thing to do.

  • [x]

    (FIXED) I get different results when running tests versus checking a file manually:

    $ pytest mypy/test/testcheck.py::TypeCheckSuite::check-expressions.test

Expected:
Actual:
main:15: error: Expression is of type "Tuple[int, ...]", not "Size" (diff)
main:16: error: Expression is of type "Tuple[int, ...]", not "Size" (diff)

    versus

    $ mypy example.py
example.py:4: error: Signature of "__add__" incompatible with supertype "tuple"  [override]
example.py:4: note:      Superclass:
example.py:4: note:          @overload
example.py:4: note:          def __add__(self, tuple[int, ...], /) -> tuple[int, ...]
example.py:4: note:          @overload
example.py:4: note:          def [_T] __add__(self, tuple[_T, ...], /) -> tuple[int | _T, ...]
example.py:4: note:      Subclass:
example.py:4: note:          def __add__(self, tuple[int, ...], /) -> Size
example.py:14: error: Expression is of type "tuple[int, ...]", not "Size"  [assert-type]
example.py:15: error: Expression is of type "tuple[int, ...]", not "Size"  [assert-type]
example.py:16: error: Expression is of type "tuple[int, ...]", not "Size"  [assert-type]
example.py:18: error: Expression is of type "tuple[int, ...]", not "Size"  [assert-type]
Found 5 errors in 1 file (checked 1 source file)

randolf-scholz avatar May 06 '25 17:05 randolf-scholz

Diff from mypy_primer, showing the effect of this PR on open source code:

xarray (https://github.com/pydata/xarray)
+ xarray/computation/computation.py: note: In function "_cov_corr":
+ xarray/computation/computation.py:304: error: Redundant cast to "T_DataArray"  [redundant-cast]

github-actions[bot] avatar May 06 '25 18:05 github-actions[bot]

I get different results when running tests versus checking a file manually:

Likely typeshed definition of tuple.__{r,}add__ is more complex than what's in our fixture ([buitlins fixtures/tuple.pyi]). That fixture is used, khm, everywhere, so consider copying it and adding real __add__ and __radd__ to tuple from typeshed (mypy/typeshed/stdlib/builtins.pyi) in the new fixture.

Conditional check

Your new version looks reasonable: we want to exclude some kinds of Instances that shouldn't be treated that way, but for any other combination runtime coverage is sufficient. This sounds good, though perhaps more tests are needed (typevar in LHS/RHS? TypeVarTuple? TypedDict/dict?) to make sure we don't actually add more than necessary.

sterliakov avatar May 06 '25 18:05 sterliakov

@sterliakov I added a tuple-typeshed.pyi fixture, now manual and test-time checks give the same result.

randolf-scholz avatar May 07 '25 13:05 randolf-scholz

Diff from mypy_primer, showing the effect of this PR on open source code:

xarray (https://github.com/pydata/xarray)
+ xarray/computation/computation.py: note: In function "_cov_corr":
+ xarray/computation/computation.py:304: error: Redundant cast to "T_DataArray"  [redundant-cast]

github-actions[bot] avatar May 07 '25 13:05 github-actions[bot]

Diff from mypy_primer, showing the effect of this PR on open source code:

typeshed-stats (https://github.com/AlexWaygood/typeshed-stats): 1.21x slower (82.6s -> 99.6s in single noisy sample)

prefect (https://github.com/PrefectHQ/prefect): 1.20x slower (75.6s -> 90.5s in single noisy sample)

materialize (https://github.com/MaterializeInc/materialize): 1.09x slower (135.7s -> 147.4s in single noisy sample)

scikit-build-core (https://github.com/scikit-build/scikit-build-core): 1.12x slower (46.4s -> 51.7s in single noisy sample)

discord.py (https://github.com/Rapptz/discord.py): 1.16x faster (313.2s -> 268.8s in single noisy sample)

xarray (https://github.com/pydata/xarray)
+ xarray/computation/computation.py: note: In function "_cov_corr":
+ xarray/computation/computation.py:302: error: Redundant cast to "T_DataArray"  [redundant-cast]

zulip (https://github.com/zulip/zulip): 1.05x faster (329.8s -> 313.9s in single noisy sample)

github-actions[bot] avatar Jun 03 '25 09:06 github-actions[bot]

1.20x slower (75.6s -> 90.5s in single noisy sample)

Uh, that seems bad. Let's see if it goes any faster if we switch around the conditions in and, assuming that covers_at_runtime is slow.

randolf-scholz avatar Jun 03 '25 09:06 randolf-scholz

Diff from mypy_primer, showing the effect of this PR on open source code:

antidote (https://github.com/Finistere/antidote): 1.55x slower (61.4s -> 95.3s in single noisy sample)

prefect (https://github.com/PrefectHQ/prefect): 1.06x faster (71.9s -> 67.8s in single noisy sample)

mypy (https://github.com/python/mypy): 1.07x slower (100.0s -> 106.8s in single noisy sample)

xarray (https://github.com/pydata/xarray)
+ xarray/computation/computation.py: note: In function "_cov_corr":
+ xarray/computation/computation.py:302: error: Redundant cast to "T_DataArray"  [redundant-cast]

github-actions[bot] avatar Jun 03 '25 09:06 github-actions[bot]

Diff from mypy_primer, showing the effect of this PR on open source code:

colour (https://github.com/colour-science/colour)
- colour/notation/munsell.py:1992: error: Incompatible return value type (got "int | floating[_16Bit] | floating[_32Bit] | floating[_32Bit | _64Bit]", expected "float")  [return-value]
+ colour/notation/munsell.py:1992: error: Incompatible return value type (got "int | floating[_16Bit] | floating[_32Bit] | floating[_64Bit]", expected "float")  [return-value]

xarray (https://github.com/pydata/xarray)
+ xarray/computation/computation.py: note: In function "_cov_corr":
+ xarray/computation/computation.py:302: error: Redundant cast to "T_DataArray"  [redundant-cast]

github-actions[bot] avatar Jun 13 '25 13:06 github-actions[bot]

Diff from mypy_primer, showing the effect of this PR on open source code:

xarray (https://github.com/pydata/xarray)
+ xarray/computation/computation.py: note: In function "_cov_corr":
+ xarray/computation/computation.py:302: error: Redundant cast to "T_DataArray"  [redundant-cast]

github-actions[bot] avatar Jul 15 '25 16:07 github-actions[bot]

Diff from mypy_primer, showing the effect of this PR on open source code:

xarray (https://github.com/pydata/xarray)
+ xarray/computation/computation.py: note: In function "_cov_corr":
+ xarray/computation/computation.py:302: error: Redundant cast to "T_DataArray"  [redundant-cast]

github-actions[bot] avatar Jul 19 '25 22:07 github-actions[bot]

Diff from mypy_primer, showing the effect of this PR on open source code:

xarray (https://github.com/pydata/xarray)
+ xarray/computation/computation.py: note: In function "_cov_corr":
+ xarray/computation/computation.py:302: error: Redundant cast to "T_DataArray"  [redundant-cast]

github-actions[bot] avatar Aug 04 '25 17:08 github-actions[bot]

Diff from mypy_primer, showing the effect of this PR on open source code:

scipy-stubs (https://github.com/scipy/scipy-stubs)
-   File "/tmp/mypy_primer/mypy_old/venv/bin/mypy", line 7, in <module>
+   File "/tmp/mypy_primer/mypy_new/venv/bin/mypy", line 7, in <module>
-   File "/tmp/mypy_primer/mypy_old/venv/lib/python3.13/site-packages/mypy/__main__.py", line 15, in console_entry
+   File "/tmp/mypy_primer/mypy_new/venv/lib/python3.13/site-packages/mypy/__main__.py", line 15, in console_entry
-   File "/tmp/mypy_primer/mypy_old/venv/lib/python3.13/site-packages/mypy/main.py", line 127, in main
+   File "/tmp/mypy_primer/mypy_new/venv/lib/python3.13/site-packages/mypy/main.py", line 127, in main
-   File "/tmp/mypy_primer/mypy_old/venv/lib/python3.13/site-packages/mypy/main.py", line 211, in run_build
+   File "/tmp/mypy_primer/mypy_new/venv/lib/python3.13/site-packages/mypy/main.py", line 211, in run_build
-   File "/tmp/mypy_primer/mypy_old/venv/lib/python3.13/site-packages/mypy/build.py", line 196, in build
+   File "/tmp/mypy_primer/mypy_new/venv/lib/python3.13/site-packages/mypy/build.py", line 196, in build
-   File "/tmp/mypy_primer/mypy_old/venv/lib/python3.13/site-packages/mypy/build.py", line 272, in _build
+   File "/tmp/mypy_primer/mypy_new/venv/lib/python3.13/site-packages/mypy/build.py", line 272, in _build
-   File "/tmp/mypy_primer/mypy_old/venv/lib/python3.13/site-packages/mypy/build.py", line 2946, in dispatch
+   File "/tmp/mypy_primer/mypy_new/venv/lib/python3.13/site-packages/mypy/build.py", line 2946, in dispatch
-   File "/tmp/mypy_primer/mypy_old/venv/lib/python3.13/site-packages/mypy/build.py", line 3346, in process_graph
+   File "/tmp/mypy_primer/mypy_new/venv/lib/python3.13/site-packages/mypy/build.py", line 3346, in process_graph
-   File "/tmp/mypy_primer/mypy_old/venv/lib/python3.13/site-packages/mypy/build.py", line 3475, in process_stale_scc
+   File "/tmp/mypy_primer/mypy_new/venv/lib/python3.13/site-packages/mypy/build.py", line 3475, in process_stale_scc
-   File "/tmp/mypy_primer/mypy_old/venv/lib/python3.13/site-packages/mypy/build.py", line 2493, in write_cache
+   File "/tmp/mypy_primer/mypy_new/venv/lib/python3.13/site-packages/mypy/build.py", line 2493, in write_cache
-   File "/tmp/mypy_primer/mypy_old/venv/lib/python3.13/site-packages/mypy/cache.py", line 28, in __init__
+   File "/tmp/mypy_primer/mypy_new/venv/lib/python3.13/site-packages/mypy/cache.py", line 28, in __init__

xarray (https://github.com/pydata/xarray)
+ xarray/computation/computation.py: note: In function "_cov_corr":
+ xarray/computation/computation.py:302: error: Redundant cast to "T_DataArray"  [redundant-cast]

AutoSplit (https://github.com/Toufool/AutoSplit)
-   File "/tmp/mypy_primer/mypy_old/venv/bin/mypy", line 7, in <module>
+   File "/tmp/mypy_primer/mypy_new/venv/bin/mypy", line 7, in <module>
-   File "/tmp/mypy_primer/mypy_old/venv/lib/python3.13/site-packages/mypy/__main__.py", line 15, in console_entry
+   File "/tmp/mypy_primer/mypy_new/venv/lib/python3.13/site-packages/mypy/__main__.py", line 15, in console_entry
-   File "/tmp/mypy_primer/mypy_old/venv/lib/python3.13/site-packages/mypy/main.py", line 127, in main
+   File "/tmp/mypy_primer/mypy_new/venv/lib/python3.13/site-packages/mypy/main.py", line 127, in main
-   File "/tmp/mypy_primer/mypy_old/venv/lib/python3.13/site-packages/mypy/main.py", line 211, in run_build
+   File "/tmp/mypy_primer/mypy_new/venv/lib/python3.13/site-packages/mypy/main.py", line 211, in run_build
-   File "/tmp/mypy_primer/mypy_old/venv/lib/python3.13/site-packages/mypy/build.py", line 196, in build
+   File "/tmp/mypy_primer/mypy_new/venv/lib/python3.13/site-packages/mypy/build.py", line 196, in build
-   File "/tmp/mypy_primer/mypy_old/venv/lib/python3.13/site-packages/mypy/build.py", line 272, in _build
+   File "/tmp/mypy_primer/mypy_new/venv/lib/python3.13/site-packages/mypy/build.py", line 272, in _build
-   File "/tmp/mypy_primer/mypy_old/venv/lib/python3.13/site-packages/mypy/build.py", line 2946, in dispatch
+   File "/tmp/mypy_primer/mypy_new/venv/lib/python3.13/site-packages/mypy/build.py", line 2946, in dispatch
-   File "/tmp/mypy_primer/mypy_old/venv/lib/python3.13/site-packages/mypy/build.py", line 3346, in process_graph
+   File "/tmp/mypy_primer/mypy_new/venv/lib/python3.13/site-packages/mypy/build.py", line 3346, in process_graph
-   File "/tmp/mypy_primer/mypy_old/venv/lib/python3.13/site-packages/mypy/build.py", line 3475, in process_stale_scc
+   File "/tmp/mypy_primer/mypy_new/venv/lib/python3.13/site-packages/mypy/build.py", line 3475, in process_stale_scc
-   File "/tmp/mypy_primer/mypy_old/venv/lib/python3.13/site-packages/mypy/build.py", line 2493, in write_cache
+   File "/tmp/mypy_primer/mypy_new/venv/lib/python3.13/site-packages/mypy/build.py", line 2493, in write_cache
-   File "/tmp/mypy_primer/mypy_old/venv/lib/python3.13/site-packages/mypy/cache.py", line 28, in __init__
+   File "/tmp/mypy_primer/mypy_new/venv/lib/python3.13/site-packages/mypy/cache.py", line 28, in __init__

github-actions[bot] avatar Sep 16 '25 13:09 github-actions[bot]

Diff from mypy_primer, showing the effect of this PR on open source code:

xarray (https://github.com/pydata/xarray)
+ xarray/computation/computation.py: note: In function "_cov_corr":
+ xarray/computation/computation.py:302: error: Redundant cast to "T_DataArray"  [redundant-cast]

github-actions[bot] avatar Oct 10 '25 16:10 github-actions[bot]