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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update error message for incompatible args #6796

Merged

Conversation

@matys18
Copy link
Contributor

commented May 7, 2019

Closes #5025 馃洜

Updated the error message that is displayed when the argument types of a method in a subclass are incompatible with the arguments of the same method in the superclass.

Since the output message has changed there are quite a few broken tests. I would like to review and agree on the wording before changing them.

# sample.py

class A:

    def kek(self, lol: int = 0) -> None:
        print("Kek in A")


class B(A):

    def kek(self, haha: str = "") -> None:
        super().kek(1)
        print("Kek in B")

Output before:

mypy sample.py
sample.py:11: error: Argument 1 of "kek" incompatible with supertype "A"

Output now:

mypy sample.py
sample.py:11: error: Argument 1 of "kek" is incompatible with supertype "A"; supertype defines the argument type as "int"

As pointed out by @ilevkivskyi incompatible return type error reporting could also benefit from the same treatment. Will enter a followup issue once this is merged.

@ilevkivskyi
Copy link
Collaborator

left a comment

The general idea looks good. It looks like you forgot to add/update tests however. Also there is one comment below.

mypy/messages.py Outdated Show resolved Hide resolved
Update error message for incompatible args
Updated the error message that is displayed when the argument types
of a method in a subclass are incompatible with the arguments of
the same method in the superclass.

@matys18 matys18 force-pushed the matys18:5025-submethod-arg-types-error-msg branch from 90d1422 to 3533a49 May 7, 2019

Update test data
Updated the test data to reflect the error message changes.
@ilevkivskyi
Copy link
Collaborator

left a comment

Thanks for updates! LGTM now.

@ilevkivskyi ilevkivskyi merged commit 56afa92 into python:master May 8, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can鈥檛 perform that action at this time.