Special Method Overload Resolution

[llchan]

• Are you reporting a bug, or opening a feature request? bug
• Please insert below the code you are checking with mypy,
  or a mock-up repro if the source is private. We would appreciate
  if you try to simplify your case to a minimal repro.

from typing import overload

class Expr: ...

class Foo:
    @overload  # type: ignore
    def __eq__(self, other: 'Foo') -> 'Expr': ...
    @overload
    def __eq__(self, other: str) -> 'Expr': ...

foo: Foo

reveal_type(foo == 1.0)

• What is the actual behavior/output?

main.pyi:17: error: Revealed type is 'builtins.bool'

• What is the behavior/output you expect?

main.pyi:17: error: No overload variant of "__eq__" of "Foo" matches argument type "float"
main.pyi:17: note: Possible overload variants:
main.pyi:17: note:     def __eq__(self, Foo) -> Expr
main.pyi:17: note:     def __eq__(self, str) -> Expr

Since Foo.__eq__ exists, it will be called for all equality comparisons at runtime, and it will not fall back to float.__eq__ unless it returns NotImplemented. Since none of the overload variants do that, it should never happen and this comparison should be a type error.

My current workaround is to include a Any overload that returns NoReturn, but that's not ideal since it defers the error at the call site to a "if I get lucky it will hit an error downstream" situation. Some of that could be addressed with more precise NoReturn semantics, but as it stands right now there are still some discussions/issues surrounding that, and it also feels clunky to have to explicitly include an overload for type errors.

In any case, I think special method overload resolution needs some fine tuning, and it should not mix-and-match the left/right operand's methods unless an overload variant is allowed to return type(NotImplemented) (which, btw, needs some type tightening in typeshed, but I'll file that over there).

• What are the versions of mypy and Python you are using?
  0.710+dev.bb7dbd5afd84656a62311e5f69a1cef6d06466bc
• Do you see the same issue after installing mypy from Git master?
  yes

[Michael0x2a]

I think the root cause here is actually due to the # type: ignore. In short, object.__eq__(...) is supposed to be able to accept any value and return any bools. (The special operator methods are also assumed to be capable of returning NotImplemented at any arbitrary point.)

So, when the subclass doesn't use the same signature (doesn't respect Liskov), you get inconsistencies like this.

Unfortunately, I'm not sure if there's a good workaround for this: the signature for object.__eq__ was defined in a way that makes conventional definitions easy, but doesn't leave a lot of wiggle room for these sorts of DSLs.

Probably there's a better way of handling this, but it's somewhat unclear to me what that way would be. For example, adjusting mypy to do this fallback behavior only when NotImplementedType is returned probably won't be sufficient: we'd also almost certainly need to adjust the signature of object.__eq__ in typeshed in some way. Finding a way of doing this that doesn't disrupt existing user code (or at least minimizes it) is likely non-trivial.

[llchan]

That's what I thought initially too, that the type ignore was causing something funny to happen with overload resolution where it would inherit a partial type signature from the base class. However, I did some testing and I think the example below makes it more clear that it's using the right operand's __eq__:

from typing import overload

class Expr: ...

class Foo:
    @overload  # type: ignore
    def __eq__(self, other: 'Foo') -> 'Expr': ...
    @overload
    def __eq__(self, other: str) -> 'Expr': ...

class Bar:
    def __eq__(self, other: object) -> int: ...

foo: Foo
bar: Bar

reveal_type(foo == bar)  # Revealed type is 'builtins.int'

Seems to me that these comparisons try overloads from the left and then try overloads from the right, rather than checking for the existence of the special method on the left and only looking at the right if the matching left signature returns NotImplemented.

[gvanrossum]

NotImplemented is defined as having type Any. This is intentional, otherwise the signature of nearly every binary dunder method would have to be an ugly union with NotImplemented. Therefore it can be returned from anything without violating the type system. So mypy assumes that there is always a possibility that the left operand returns NotImplemented, and hence it always takes the right operand's type into account.

Now, I do understand there's a real problem here -- mypy and typeshed currently aren't built to support overloading __eq__ to return a non-bool, because it is defined so in typeshed for class object.

I don't see an easy way out -- in particular because in general == can be used to compare any two objects, and is implicitly used by other operations like in and dict key access. Sorry.

[Michael0x2a]

I think the example below makes it more clear that it's using the right operand's eq:

The reason it's doing this is because mypy is assuming that the left operand overrode __eq__ correctly. My point is that since the code violates this assumption, the resulting behavior of mypy is no longer guaranteed to be correct.

[llchan]

I agree that this will not be an easy ticket to solve, and I expect it will take some time to figure out how to address this with a reasonable upgrade path. That said, I think it's important for a type checker to value correctness over brevity, and that's especially true in this case since it's not a self-contained false positive that can be hidden away by a # type: ignore. The incorrectly-inferred type can propagate and lead to uncaught type issues downstream, and we should move towards fixing that, even if it's hard and takes a while to figure out.

Now before we go further, I want to make sure I have my understanding correct:

• NotImplemented is an instance of Any to keep binary dunder signatures clean
• The mypy checker looks for the existence of a __eq__ method, and if it exists it assumes it may always return NotImplemented, since that's an instance of Any
• Therefore, it's always possible for the right operand's __eq__ to be invoked, and its signature(s) are used following the left operand's.

Is that a reasonable summary?

[llchan]

For what it's worth, pyre and pyright both emit an error:

pyre:

main.py:17:7 Incompatible parameter type [6]: Expected `str` for 1st anonymous parameter to call `main.Foo.__eq__` but got `Bar`.

pyright:

Operator '==' not supported for types 'Foo' and 'Bar' (17, 1)

[ilevkivskyi]

I am with @Michael0x2a here. After an ignored error, we try to avoid further false negatives on the best effort basis (unlike subsequent false positives, these should be always avoided if the original error is ignored). I am therefore proposing to close this issue.

[llchan]

I understand the current state of things (type ignore causing assumptions to be broken), and I can understand decreasing the issue priority since it's likely to take a bit of reworking to fix. However, I don't think this should be closed, since it's a real bug with a reliable repro and no solution. Maybe stick a help-wanted label on it, to indicate you guys arent actively tackling it, but it's still an unsolved issue?

I can try to poke around. Don't want to be one of those people whole files issues but doesnt contribute PRs. The mypy codebase is a lot to take in though, so don't hold your breath.

[Michael0x2a]

I have no opinion on whether this issue should be open or closed, but I do want to clarify that I don't think this is a bug: mypy detected the root error and your code samples silenced it. And as Ivan said, mypy operates on a best-effort principle once an error is silenced and doesn't guarantee it'll identify subsequent "downstream" errors. In that regard, everything is behaving as expected.

Rather, I'd characterize this as in part a usability issue and in part a feature request:

1. While mypy did correctly detect that the code was not type safe, I do agree it'd be nice if it inferred types using the incorrect definition of __eq__ instead of the expected definition. It wouldn't change correctness (since the error was already identified), but it would probably improve usability in cases like this.

2. A little more broadly, this is a feature request. Basically, mypy assumes that any code defining a custom operator methods are following Python's standard data model (__eq__ returns bool, operator methods return NotImplemented when given objects they can't handle...). This assumption makes sense since most objects follow this model and builtins such as dict or set assume that they do. But it would be nice if we could figure out a way to broaden the type system so that it can also accommodate non-standard operator definitions. It's not obvious to anybody how to actually do this though.

Actually, I suppose there is one thing here I'd count as a bug: mypy doesn't enforce that operator methods correctly use NotImplemented. If mypy expects operator methods to follow Python's standard data model, I think it also makes sense to try and actually enforce this.

[llchan]

Right, I think that's sort of the crux of the short-term issue: mypy always implies Union[<return type>, NotImplemented] on these methods rather than implying it once at the object level. If the user decides to override it with a type ignore, it should respect the user's return type and not inject a NotImplemented if it's not possible for that to occur. The user should include NotImplementedType (aka Any, for the time being) in their return union if that's a possible outcome.

Longer-term, I agree this is pretty nontrivial feature request to broaden the special methods so they support a wider range of use cases. The docs say

By convention, False and True are returned for a successful comparison. However, these methods can return any value, so if the comparison operator is used in a Boolean context (e.g., in the condition of an if statement), Python will call bool() on the value to determine if the result is true or false.

so object's subclasses should be allowed to return anything (whether automatic or via a special decorator or marker), and specific subclass implementations should overload where appropriate to return bool/NotImplemented. A lot of painstaking work to get that into typeshed though, so I understand the reluctance :)