"override" check causes false positive error

[jab]

Bug Report

mypy's "override" check is erroneously flagging the type hints for the pop() method in the following code:

from collections import UserDict
from enum import Enum
import typing as t


KT = t.TypeVar('KT')
VT = t.TypeVar('VT')


class MissingT(Enum):
    """Sentinel used to represent none/missing when None itself can't be used."""
    MISSING = 'MISSING'


MISSING = MissingT.MISSING


DT = t.TypeVar('DT')  #: for default arguments
ODT = t.Union[DT, MissingT]


class mydict(UserDict[KT, VT], t.MutableMapping[KT, VT]):
    @t.overload
    def pop(self, __key: KT) -> VT: ...
    @t.overload
    def pop(self, __key: KT, __default: t.Union[VT, DT]) -> t.Union[VT, DT]: ...

    def pop(self, key: KT, default: ODT[DT] = MISSING) -> t.Union[VT, DT]:
        ...

See https://mypy-play.net/?mypy=latest&python=3.10&gist=eb527d76a58888350b4038002816923f for a live demo.

Other type checkers, such as Pyright, flag no error here. But mypy fails with the following:

main.py:23: error: Signature of "pop" incompatible with supertype "MutableMapping"
main.py:23: note:      Superclass:
main.py:23: note:          @overload
main.py:23: note:          def pop(self, KT) -> VT
main.py:23: note:          @overload
main.py:23: note:          def [_T] pop(self, KT, Union[VT, _T] = ...) -> Union[VT, _T]
main.py:23: note:      Subclass:
main.py:23: note:          @overload
main.py:23: note:          def pop(self, KT) -> VT
main.py:23: note:          @overload
main.py:23: note:          def [DT] pop(self, KT, Union[VT, DT]) -> Union[VT, DT]
Found 1 error in 1 file (checked 1 source file)

Comparing the superclass to the subclass, mypy apparently thinks that

def [_T] pop(self, KT, Union[VT, _T] = ...) -> Union[VT, _T]

is incompatible with

def [DT] pop(self, KT, Union[VT, DT]) -> Union[VT, DT]

even though these should be compatible.

Your Environment

• Mypy version used: 0.941 (latest)
• Mypy command-line flags: none
• Python version used: 3.10 (latest)

[KotlinIsland]

Here is your issue minified:
without overloads
with overloads

Which looks like correct analysis to me.

Why not annotate __default as a = parameter?

[jab]

Thanks @KotlinIsland, I missed that tiny " = ..." difference in that error message, and instead zeroed in on the more noticeable "_T" vs. "DT" difference, even though that difference shouldn't matter. And since Pyright flagged no issues with the same code, it led me to open this. But I guess this is really a false negative bug in Pyright?

If so, then the only thing I'd propose mypy do differently here would be to improve the error message to point out the actually-problematic part(s) of the difference, e.g.

main.py:23: error: Signature of "pop" incompatible with supertype "MutableMapping" in second overload
main.py:23: note:      Superclass:
main.py:23: note:          @overload
main.py:23: note:          def pop(self, KT) -> VT
main.py:23: note:          @overload
main.py:23: note:          def [_T] pop(self, KT, Union[VT, _T] = ...) -> Union[VT, _T]
                                                                ^^^^^
main.py:23: note:      Subclass:
main.py:23: note:          @overload
main.py:23: note:          def pop(self, KT) -> VT
main.py:23: note:          @overload
main.py:23: note:          def [DT] pop(self, KT, Union[VT, DT]) -> Union[VT, DT]
Found 1 error in 1 file (checked 1 source file)

Note the addition of "in second overload" to the error message, as well as the all-important ^ arrows pointing out the problematic part(s) of the difference.

[jab]

Oh, perhaps the most problematic thing here is that the mypy error message only mentions incompatibility between the subclass and superclass. But there is no incompatibility in the actual implementation: The version of pop() that gets used at runtime does give the __default argument a default value. i.e. There is no runtime error here. It's just that the second overload forgot to include the "= ..." in the type hint. So perhaps the difference between the subclass's second overload and its actual runtime signature is what mypy should be pointing out, instead of or in addition to the current error message?

And given that, I'm not sure I'd call this a false negative bug in Pyright, as much as neglecting to enforce pedantic enough overloads. Thoughts?

❯ cat foo.py
import typing as t

T = t.TypeVar("T")

class A(t.Generic[T]):
    @t.overload
    def pop(self) -> T: ...
    @t.overload
    def pop(self, d: T=...) -> T: ...
    def pop(self, d: T=...) -> T: ...

class B(A[T]):
    @t.overload
    def pop(self) -> T: ...
    @t.overload
    def pop(self, d: T) -> T: ...      # This...
    def pop(self, d: T=...) -> T: ...  # ...doesn't match this. This line makes the subclass compatible with the superclass, but mypy ignores this line?

b: A[int] = B()

b.pop() # NOT a runtime error


❯ mypy foo.py
fn.py:13: error: Signature of "pop" incompatible with supertype "A"
fn.py:13: note:      Superclass:
fn.py:13: note:          @overload
fn.py:13: note:          def pop(self) -> T
fn.py:13: note:          @overload
fn.py:13: note:          def pop(self, d: T = ...) -> T
fn.py:13: note:      Subclass:
fn.py:13: note:          @overload
fn.py:13: note:          def pop(self) -> T
fn.py:13: note:          @overload
fn.py:13: note:          def pop(self, d: T) -> T
Found 1 error in 1 file (checked 1 source file)


❯ pyright foo.py
Loading configuration file at /private/tmp/pyrightconfig.json
Assuming Python version 3.10
Assuming Python platform Darwin
Auto-excluding **/node_modules
Auto-excluding **/__pycache__
Auto-excluding **/.*
stubPath /private/tmp/typings is not a valid directory.
Searching for source files
Found 1 source file
0 errors, 0 warnings, 0 informations
Completed in 0.589sec

[KotlinIsland]

Annoyingly mypy isn't very strict regarding overload implementations #12401

[erictraut]

But I guess this is really a false negative bug in Pyright?

Yeah, this was a missing check in Pyright. This will be addressed in the next release.

[jab]

Thanks @erictraut! (Anyone ever tell you your follow-up is amazing?:)

On the mypy side, should this issue be repurposed for improving the error message?

[KotlinIsland]

Probably better to raise a new one

[jab]

@erictraut writes:

But I guess this is really a false negative bug in Pyright?

Yeah, this was a missing check in Pyright. This will be addressed in the next release.

Since there's now been a new release of Pyright, I retried the reproducer in #12390 (comment) with latest Pyright, and it still emitted no errors. I didn't see a corresponding fix in https://github.com/microsoft/pyright/commits or issue in the tracker. Happy to create one if that'd be helpful, and hopefully I haven't missed something.

(Before raising a new issue to improve mypy's output for this type of error, I wanted to see what Pyright's output would be like for this type of error first, in case Pyright comes up with any other improvements I didn't think of. In the meantime, I'll close this issue since this isn't actually a false positive error in mypy.)

[jab]

And another thing before I raise a followup mypy issue for this, can anyone respond to this question? It seems this error is more a case of "overloads don't add up to properly match function's runtime signature" (due to forgetting to give the kwarg a default value in the corresponding overload), more-so than the subclass's method matching the superclass's?

[KotlinIsland]

@jab Mypy doesn't consider the implementation when comparing the types, the overloads are the only source of information:
https://mypy-play.net/?mypy=latest&python=3.10&flags=strict&gist=2dbc5a9476775301804ce281a6883eed

Which isn't the best in my opinion and leads to situation like this.