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

@overload-ing method of parent class without actual implementation #86978

Closed
chaim422 mannequin opened this issue Jan 3, 2021 · 13 comments
Closed

@overload-ing method of parent class without actual implementation #86978

chaim422 mannequin opened this issue Jan 3, 2021 · 13 comments
Labels
3.10 only security fixes topic-typing type-feature A feature request or enhancement

Comments

@chaim422
Copy link
Mannequin

chaim422 mannequin commented Jan 3, 2021

BPO 42812
Nosy @gvanrossum, @uriyyo

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2021-01-03.05:51:32.158>
labels = ['type-feature', '3.10']
title = '@overload-ing method of parent class without actual implementation'
updated_at = <Date 2021-01-12.00:32:35.122>
user = 'https://bugs.python.org/chaim422'

bugs.python.org fields:

activity = <Date 2021-01-12.00:32:35.122>
actor = 'gvanrossum'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = []
creation = <Date 2021-01-03.05:51:32.158>
creator = 'chaim422'
dependencies = []
files = []
hgrepos = []
issue_num = 42812
keywords = []
message_count = 12.0
messages = ['384255', '384265', '384283', '384284', '384285', '384286', '384287', '384288', '384289', '384290', '384292', '384871']
nosy_count = 3.0
nosy_names = ['gvanrossum', 'uriyyo', 'chaim422']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue42812'
versions = ['Python 3.10']

@chaim422
Copy link
Mannequin Author

chaim422 mannequin commented Jan 3, 2021

Why should @overload need to be followed by an implementation when an implementation already exists in the parent class?

Illustrative example:

class Parent:
    def foo(**kwargs):
        """Argument names of foo vary depending on the child class."""

class Child(Parent):
    @overload foo(a, b): ...
    
Raises:

"NotImplementedError: You should not call an overloaded function. A series of @overload-decorated functions outside a stub module should always be followed by an implementation that is not @overload-ed."

@chaim422 chaim422 mannequin added 3.10 only security fixes type-feature A feature request or enhancement labels Jan 3, 2021
@uriyyo
Copy link
Member

uriyyo commented Jan 3, 2021

The purpose of @overload is quite different. I believe you thought that this is smth like @override in Java world but it different.

Basically, the correct usage of @overaload is:

@overload
def process(response: None) -> None:
    ...
@overload
def process(response: int) -> tuple[int, str]:
    ...
@overload
def process(response: bytes) -> str:
    ...
def process(response):
    <actual implementation>

Please, follow this link for more information https://docs.python.org/3/library/typing.html#typing.overload

@chaim422
Copy link
Mannequin Author

chaim422 mannequin commented Jan 3, 2021

"The purpose of @overload is quite different." So, this would overload the @overload decorator. Hmmm...

Is there a better way to accomplish this goal? What would you suggest, a new decorator?

@chaim422
Copy link
Mannequin Author

chaim422 mannequin commented Jan 3, 2021

To clarify, this is how it's being done now a dozen times in actual production code:

class Child(Parent):
    @overload foo(a, b): ...
    def overload(**kwargs):
        return super().foo(**kwargs)

The goal of this proposed enhancement is to remove two extra lines of code per Child class.

@uriyyo
Copy link
Member

uriyyo commented Jan 3, 2021

mypy will produce an error on such code:

class Parent:
    def foo(self, **kwargs):
        """Argument names of foo vary depending on the child class."""


class Child(Parent):
    @overload
    def foo(self, a, b):
        pass

    def foo(self, **kwargs):
        return super().foo(**kwargs)

Result

temp.py:10: error: Single overload definition, multiple required
temp.py:10: error: Signature of "foo" incompatible with supertype "Parent"
Found 2 errors in 1 file (checked 1 source file)

In case if you want to add an overload for method I would recommend to use such pattern:

class Parent:
    def foo(self, **kwargs):
        """Argument names of foo vary depending on the child class."""


class Child(Parent):
    @overload
    def foo(self, a, b):
        pass

    @overload
    def foo(self, **kwargs):
        pass

    def foo(self, **kwargs):
        return super().foo(**kwargs)

So signature of foo will still match to parent class signature, but it will also have an overloaded variant.

@chaim422
Copy link
Mannequin Author

chaim422 mannequin commented Jan 3, 2021

Interesting. PyCharm has no problem with this code. It also autocompletes the argument names for me, which is very useful, especially since there a dozen different Child classes.

Isn't "Simple is better than complex", and doesn't "...practicality beat purity"?

@uriyyo
Copy link
Member

uriyyo commented Jan 3, 2021

I think the simplest solution in your case is not to use @overload, as far as I understand you want to override the signature of base method.

This code won't produce any error when used with mypy:

class Parent:
    def foo(**kwargs):
        """Argument names of foo vary depending on the child class."""

class Child(Parent):
    def foo(self, a, b):
        pass
``

@chaim422
Copy link
Mannequin Author

chaim422 mannequin commented Jan 3, 2021

What is better?

A. Keeping Python as is, with four separate signature declarations (1x Parent, 2x @overload Child, and 1x actual signature in Child), and a second method call overhead at runtime (to satisfy mypy as it exists now).

--or--

B. Simplify Python to allow just two signature declarations and no extra overhead?

--or--

C. Something else? Please specify.

@chaim422
Copy link
Mannequin Author

chaim422 mannequin commented Jan 3, 2021

In your example, does Child foo call Parent foo? Because the intent is to use the parent's foo method.

@uriyyo
Copy link
Member

uriyyo commented Jan 3, 2021

What is better?

Sorry, I can't answer this question.

I am a regular python user and I just tried to help with your issue.

I believe we should wait for someone from core team to answer this question.

In your example, does Child foo call Parent foo? Because the intent is to use the parent's foo method.

Sorry, I made a mistake, it definitely should call a parent method. A correct example will look like this:

class Parent:
    def foo(self, **kwargs):
        """Argument names of foo vary depending on the child class."""


class Child(Parent):
    def foo(self, a, b):
        super().foo(a=a, b=b)

But, the example above require more code to write, so a better option will be:

class Parent:
    def foo(self, **kwargs):
        """Argument names of foo vary depending on the child class."""


class Child(Parent):
    @overload
    def foo(self, a, b):
        pass

    @overload
    def foo(self, **kwargs):
        pass

    def foo(self, **kwargs):
        return super().foo(**kwargs)

I am not sure is it the perfect solution to solve your issue.

Let's wait for someone from core team, so we can hear their opinion.

@chaim422
Copy link
Mannequin Author

chaim422 mannequin commented Jan 3, 2021

Thanks for your perspective.

To summarize here is how my proposed enhancement might look in practice:

class Parent:
    def foo(self, **kwargs):
        """Argument names of foo vary depending on the child class."""


class Child(Parent):
    @overload
    def foo(self, a, b):
        pass

@gvanrossum
Copy link
Member

I hesitate to add anything because you are exposing so much confusion. May I suggest that you ask about this on a user group first before proposing a new feature? One place that makes sense given that this is a type system feature would be this Gitter channel: https://gitter.im/python/typing

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
@AlexWaygood
Copy link
Member

I hesitate to add anything because you are exposing so much confusion. May I suggest that you ask about this on a user group first before proposing a new feature? One place that makes sense given that this is a type system feature would be this Gitter channel: https://gitter.im/python/typing

I agree with @gvanrossum. I'm closing this issue, as I don't think this issue tracker is an appropriate place for this conversation to be held. Please consider asking your question at https://github.com/python/typing/discussions or on StackOverflow.

Cc. @chaim422

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.10 only security fixes topic-typing type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

3 participants