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

"Signature of X incompatible with supertype Y" message could be more specific when return types differ #3818

Closed
OddBloke opened this issue Aug 9, 2017 · 2 comments

Comments

@OddBloke
Copy link
Contributor

OddBloke commented Aug 9, 2017

When investigating a (non-)issue in #3805, I ended up slightly more confused than I could have been due to the error messages I was seeing. When I run mypy against:

from typing import Mapping, Iterable, Tuple, Optional


class MyDict(Mapping):

    def items(self, irrelevant_arg: str = "foo") -> Iterable[Tuple[str, str]]: ...

I get:

test.pyi:6: error: Signature of "items" incompatible with supertype "Mapping"

As Mapping.items() doesn't take any arguments, my initial assumption was that the issue here was the argument. However, the issue is actually the return type; mypy is totally happy with:

    def items(self, irrelevant_arg: str = "foo") -> AbstractSet[Tuple[str, str]]: ...

and if the function hadn't had any arguments:

    def items(self) -> Iterable[Tuple[str, str]]: ...

then mypy would actually have told me this:

test.pyi:6: error: Return type of "items" incompatible with supertype "Mapping"

Looking at https://github.com/python/mypy/blob/master/mypy/checker.py#L1108-L1145, I think that the return type checking could move out an if-statement and potentially affect the error output in both the same-args and different-number-of-args cases.

@OddBloke
Copy link
Contributor Author

OddBloke commented Aug 9, 2017

Very roughly, to hit the return check in more cases:

diff --git a/mypy/checker.py b/mypy/checker.py
index ca9cf379..e086b226 100644
--- a/mypy/checker.py
+++ b/mypy/checker.py
@@ -1108,9 +1108,7 @@ class TypeChecker(NodeVisitor[None], CheckerPluginInterface):
         if fail:
             emitted_msg = False
             if (isinstance(override, CallableType) and
-                    isinstance(original, CallableType) and
-                    len(override.arg_types) == len(original.arg_types) and
-                    override.min_args == original.min_args):
+                    isinstance(original, CallableType)):
                 # Give more detailed messages for the common case of both
                 # signatures having the same number of arguments and no
                 # overloads.
@@ -1126,12 +1124,14 @@ class TypeChecker(NodeVisitor[None], CheckerPluginInterface):
                 def erase_override(t: Type) -> Type:
                     return erase_typevars(t, ids_to_erase=override_ids)
 
-                for i in range(len(override.arg_types)):
-                    if not is_subtype(original.arg_types[i],
-                                      erase_override(override.arg_types[i])):
-                        self.msg.argument_incompatible_with_supertype(
-                            i + 1, name, name_in_super, supertype, node)
-                        emitted_msg = True
+                if (len(override.arg_types) == len(original.arg_types) and
+                        override.min_args == original.min_args):
+                    for i in range(len(override.arg_types)):
+                        if not is_subtype(original.arg_types[i],
+                                        erase_override(override.arg_types[i])):
+                            self.msg.argument_incompatible_with_supertype(
+                                i + 1, name, name_in_super, supertype, node)
+                            emitted_msg = True
 
                 if not is_subtype(erase_override(override.ret_type),
                                   original.ret_type):

gives me:

3818.pyi:6: error: Return type of "items" incompatible with supertype "Mapping"

@JukkaL
Copy link
Collaborator

JukkaL commented Aug 9, 2017

Closing as a duplicate of #3379. The PR #3410 is related.

@JukkaL JukkaL closed this as completed Aug 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants