Repository navigation
"override" check causes false positive error #12390
Description
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)
Activity
KotlinIsland commentedon Mar 21, 2022
Here is your issue minified:
without overloads
with overloads
Which looks like correct analysis to me.
Why not annotate
__defaultas a=parameter?Better workaround for python/mypy#12390
jab commentedon Mar 21, 2022
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.
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 commentedon Mar 21, 2022
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__defaultargument 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?
KotlinIsland commentedon Mar 21, 2022
Annoyingly mypy isn't very strict regarding overload implementations #12401
erictraut commentedon Mar 21, 2022
Yeah, this was a missing check in Pyright. This will be addressed in the next release.
jab commentedon Mar 21, 2022
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 commentedon Mar 21, 2022
Probably better to raise a new one
jab commentedon Mar 23, 2022
@erictraut writes:
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 commentedon Mar 23, 2022
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 commentedon Mar 24, 2022
@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.