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

default value for overloaded optional parameters #7333

Closed
crusaderky opened this issue Aug 13, 2019 · 7 comments
Closed

default value for overloaded optional parameters #7333

crusaderky opened this issue Aug 13, 2019 · 7 comments

Comments

@crusaderky
Copy link
Contributor

crusaderky commented Aug 13, 2019

Python 3.7.4, typing_extensions 3.7.4, mypy 0.720

I'm trying to use Literal and @overloadto change the result type of a function depending on a boolean switch.
This issue is not specific to Literal, but it makes it easier to produce an easy to understand real-life use case.

Everything works great as long as the parameter is mandatory:

from typing import overload, Optional
from typing_extensions import Literal


@overload
def plusone(x: Optional[int], none_to_zero: Literal[True]) -> int:
    ...


@overload
def plusone(x: Optional[int], none_to_zero: Literal[False]) -> Optional[int]:
    ...


def plusone(x, none_to_zero):
    if x is None:
        if none_to_zero:
            x = 0
        else:
            return None
    return x + 1


plusone(None, True).to_bytes(4, "little") # ok
plusone(None, False).to_bytes(4, "little") # not ok

mypy output: 25: error: Item "None" of "Optional[int]" has no attribute "to_bytes"

However, if I set a default value:

from typing import overload, Optional
from typing_extensions import Literal


@overload
def plusone(x: Optional[int], none_to_zero: Literal[True] = True) -> int:
    ...


@overload
def plusone(x: Optional[int], none_to_zero: Literal[False] = True) -> Optional[int]:
    ...


def plusone(x, none_to_zero=True):
    if x is None:
        if none_to_zero:
            x = 0
        else:
            return None
    return x + 1


plusone(None, True).to_bytes(4, "little") # ok
plusone(None, False).to_bytes(4, "little") # not ok
plusone(None).to_bytes(4, "little") # ok

mypy output:

11: error: Incompatible default for argument "none_to_zero" (default has type "Literal[True]", argument has type "Literal[False]")
25: error: Item "None" of "Optional[int]" has no attribute "to_bytes"

Workaround
Add # type: ignore on line 11. The error on line 25 is still reported.

Expected behaviour
In each of the signatures of an @overload'ed function, the allowed default values of an optional parameter should be the Union of type annotations from all the signatures.

@JelleZijlstra
Copy link
Member

You should only set the default on one of the overloads (the one that will get taken if the argument is not given). Otherwise mypy does not know which overload to use if the defaulted argument is not given.

@JohnVillalovos
Copy link

You should only set the default on one of the overloads (the one that will get taken if the argument is not given). Otherwise mypy does not know which overload to use if the defaulted argument is not given.

But then you can get an error like:

error: non-default argument follows default argument

Code I'm trying it on:

    @overload
    def http_get(
        self,
        path: str,
        query_data: Optional[Dict[str, Any]] = None,
        streamed: Literal[True],
        raw: bool = False,
        **kwargs: Any,
    ) -> requests.Response:
        ...

    @overload
    def http_get(
        self,
        path: str,
        query_data: Optional[Dict[str, Any]] = None,
        streamed: bool = False,
        raw: Literal[True] = False,
        **kwargs: Any,
    ) -> requests.Response:
        ...

    @overload
    def http_get(
        self,
        path: str,
        query_data: Optional[Dict[str, Any]] = None,
        streamed: bool = False,
        raw: bool = False,
        **kwargs: Any,
    ) -> Union[Dict[str, Any], requests.Response]:
        ...

    def http_get(
        self,
        path: str,
        query_data: Optional[Dict[str, Any]] = None,
        streamed: bool = False,
        raw: bool = False,
        **kwargs: Any,
    ) -> Union[Dict[str, Any], requests.Response]:

@stianjensen
Copy link

I just stumbled upon this as well. It seems to me that you can solve this by adding a * before the parameter that is actually optional in one of the overloads.

So, something like:

    @overload
    def http_get(
        self,
        path: str,
        query_data: Optional[Dict[str, Any]] = None,
        raw: bool = False,
        *,
        streamed: Literal[True],
        **kwargs: Any,
    ) -> requests.Response:
        ...

    @overload
    def http_get(
        self,
        path: str,
        query_data: Optional[Dict[str, Any]] = None,
        streamed: bool = False,
        *
        raw: Literal[True],
        **kwargs: Any,
    ) -> requests.Response:
        ...

    @overload
    def http_get(
        self,
        path: str,
        query_data: Optional[Dict[str, Any]] = None,
        streamed: bool = False,
        raw: bool = False,
        **kwargs: Any,
    ) -> Union[Dict[str, Any], requests.Response]:
        ...

    def http_get(
        self,
        path: str,
        query_data: Optional[Dict[str, Any]] = None,
        streamed: bool = False,
        raw: bool = False,
        **kwargs: Any,
    ) -> Union[Dict[str, Any], requests.Response]:

You will need to alternately re-order the arguments for that to work, since the required literal must be the only thing after * for each overload. Hope this helps!

@rggjan
Copy link

rggjan commented May 10, 2022

@JelleZijlstra can this issue be re-opened? Clearly the solution in #7333 (comment) doesn't work according to #7333 (comment), and #7333 (comment) seems like a work-around which probably doesn't work properly with non-keyword arguments in function calls given the *?

@JelleZijlstra
Copy link
Member

@stianjensen's workaround should work in all cases, although it may need a lot of tricky overloads.

@rggjan
Copy link

rggjan commented May 11, 2022

Hmm... can you give a working example? Testing what is described in #7333 (comment) results in:

error: Overloaded function signature 3 will never be matched: signature 2's parameter type(s) are the same or broader  [misc]
error: Overloaded function implementation does not accept all possible arguments of signature 1  [misc]
error: Overloaded function implementation does not accept all possible arguments of signature 2  [misc]

for me...

@maegul
Copy link

maegul commented Jul 14, 2022

Well @rggjan, my take away from @stianjenson (which I don't think I ever would have though of on my own ... thanks!) is that keyword only arguments is simply the cost of the ticket out of strife with overloads and default values. Personally, I'm happy to make all arguments involved in the overload variations keyword-only, as default-values no longer have any ordering requirement.

And yes, function calls will need argument keywords ... but I think avoiding that is the kind of developer laziness/easiness that really doesn't have much or any value given the cost involved.

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

6 participants