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

Decorator on method in derived class #1441

Closed
drewhaven opened this issue Apr 25, 2016 · 4 comments · Fixed by #3918
Closed

Decorator on method in derived class #1441

drewhaven opened this issue Apr 25, 2016 · 4 comments · Fixed by #3918
Assignees
Labels
bug mypy got something wrong priority-0-high

Comments

@drewhaven
Copy link

A decorator that changes the signature of a function is rejected if the old signature isn't consistent with the superclass. E.g.:

def convert_to_bool(f):
    # type: (Callable[..., int]) -> Callable[..., bool]
    def wrapper(*n, **kw):
        i = f(*n, **kw)
        return i < 10
    return wrapper

class Base(object):
    def f(self):
        # type: () -> bool
        return True

class Derived(Base):
    @convert_to_bool
    def f(self):
        # type: () -> int
        return 20

The interesting point here is that the base class doesn't use the decorator while the child does. Our particular issue is in code that uses contextlib.contextmanager, but I think we can work around it by adding the decorator to the base class.

@drewhaven
Copy link
Author

Actually, that work-around doesn't currently work. Adding the decorator to Base.f still gives the error on Derived.f: Signature of "f" incompatible with supertype "Base"

@gvanrossum gvanrossum added the bug mypy got something wrong label Apr 26, 2016
@gvanrossum
Copy link
Member

Looking at the code a bit, it looks like check_method_override(), which is supposed to check whether Derived.f is compatible with Base.f, is called with the undecorated function. I'm guessing the decorator is stripped off somewhere a little earlier -- in fact, it seems the cuprit here is visit_decorator() that just calls e.func.accept().

@gvanrossum
Copy link
Member

We really should fix this one!

@JukkaL
Copy link
Collaborator

JukkaL commented Aug 8, 2017

More examples in #2469.

JukkaL added a commit that referenced this issue Sep 5, 2017
The fix turned out to be pretty complicated, as there were a bunch of
untested method overriding scenarios which weren't quite right. I
fixed any related issues that I encountered, though I'm not certain
whether some of the issues were hidden previously by other bugs. I
also added tests for some related, previously untested scenarios.

Fixes #1441.
@gvanrossum gvanrossum changed the title Decorator on function in derived class Decorator on method in derived class Sep 8, 2017
JukkaL added a commit that referenced this issue Sep 13, 2017
The fix turned out to be pretty complicated, as there were a bunch of
untested method overriding scenarios which weren't quite right. I
fixed any related issues that I encountered, though I'm not certain
whether some of the issues were hidden previously by other bugs. I
also added tests for some related, previously untested scenarios.

Fixes #1441.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong priority-0-high
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants