-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Literal
s overlap when defaults given
#6580
Comments
But mypy is right. When you call |
In the |
The point is that without arguments the defaults take over. |
I guess maybe let me rephrase the question: how do I specify a literal keyword argument in the middle of other keyword arguments? It seems that we may need a sentinel value so we can signal to the overload to only match that signature if the keyword is explicitly present as a literal. @overload
def foo(x: int = 0, y: Literal[False] = REQUIRED) -> int: ...
@overload
def foo(x: int = 0, y: Literal[True] = REQUIRED) -> str: ...
@overload
def foo(x: int = 0, y: bool = False) -> Union[int, str]: ... The desired behavior: reveal_type(foo()) # Union[int, str]
reveal_type(foo(0, False)) # int
reveal_type(foo(0, True)) # str
reveal_type(foo(y=False)) # int
reveal_type(foo(y=True)) # str
reveal_type(foo(y=z)) # Union[int, str] |
I should mention that I'm adding types to code that must continue to support PY2 for now (I know, I know, but migrating takes time, and there are many downstream users), so making these keyword-only with the star is not an option. |
You don't have to provide a default to make something a keyword argument.
can still be called as
With overloading I would recommend overloading all variants with mandatory arguments, e.g.
|
My bad, my terminology was a bit imprecise. I meant a literal after one or more defaulted arguments. The |
So make two sets of overloads -- one set with |
This toy example only has one defaulted argument before the literal, but the actual function has many other defaulted arguments before, which would require If this is a wont-fix due to limited dev resources, that's understandable, I just want to make sure I'm getting the proposal across. EDIT: was only thinking positional calls. it would need at least 2^(N+1) overloads, and also this strategy doesnt work if any other defaulted arguments have a matching type (e.g. if |
It looks like this is a real problem, and mypy could probably support this use case without requiring a huge number of overloads. I suspect that the same issue will at least occasionally come up when using literal types in library stubs. Reopening the issue, though I can't estimate when the mypy team might have resources to fix this. |
I don't get this whole issue, overload typings shouldn't include default arguments - what if someone specifies different defaults across multiple overloads? The overload decorator specification itself says that an unannotated function declaration should immediately follow the overloaded variants, hence why this should be correct: @overload
def foo(x: int, y: Literal[False]) -> int:
...
@overload
def foo(x: int, y: Literal[True]) -> str:
...
def foo(x=0, y=False):
... The only problem is that MyPy doesn't seem to look at the actual function declaration (last one) for the defaults (yet?). |
@DevilXD yes, your last sentence is essentially the crux of this problem. Without a mechanism to propagate defaults into the overloads, we need to include defaulted-ness in the overload signatures. However, this leads to "incompatible return types" issues, and furthermore we can't specify literals without defaults after defaulted arguments. The only way to solve this currently is to manually define each overload for signatures with/without those arguments set (roughly exponential). For example, in your snippet above, if there's a |
Eh, I'd say that's stupid then. You shouldn't need to define all possible variants in this case, those two should be enough. I commented on this because it seemed like everyone was trying to put the defaults into overloads themselves, which doesn't sound like a good idea to me. I found this issue due to me having a similar problem, here's my example: @overload
async def get_players(
self, player_ids: List[int], *, return_private: Literal[False] = False
) -> List[Player]:
...
@overload
async def get_players(
self, player_ids: List[int], *, return_private: Literal[True]
) -> List[Union[Player, PartialPlayer]]:
...
async def get_players(self, player_ids, *, return_private=False):
... Without that |
I agree with you in principle. The overloads shouldn't need to specify defaults (or more strongly put, maybe they shouldnt specify defaults), since the defaults are mostly* just a fancy mechanism to automatically bind values to omitted parameters. I'd maybe even take your idea a step further and say that a omitted parameter like this that defaults to True/False should map to the the corresponding literal case, rather than the bool case that I believe it would map to now. However, there may be nuances that we are overlooking. Furthermore that ship may have already sailed and it may require a PEP to change/augment the way overloads work. I'll defer to a maintainer to chime in. |
The reason why the defaults need to be specified in the overload variant list is due to the intersection of the following factors:
The consequence of these two factors is that we end up needing to encode whether an argument is positional-only, keyword-only, optional, mandatory, etc... in the overload variant signatures. Regarding the If the objection is that it feels redundant to have to actually repeat the default value, you can use
(Mypy understands the overloads disappear at runtime and so well let Regarding the original posted issue -- it seems the crux of the problem there is really that Python 2 doesn't support keyword-only arguments, and you sort of need them to express certain overloads in a clean and compact way? Unfortunately, I'm not really sure if what mypy can do here. We can't do the same kind of trick we did to support positional-only arguments in older versions of Python (treat any params starting with two underscores as positional only), and given that Python 2 is EOL'd, I'm not sure whether it's worth investing the time into finding a solution. One workaround might be to move any complicated overloads into a dedicated file with a stub file. Stub files can use Python 3 syntax (even if it's supposed to be hints for Python 2 code), which would sort of sidestep the problem. This solution is admittedly pretty suboptimal though: for example, you'd lose the ability for mypy to type check the implementation. Regarding Jukka's comment: I do think it's worth auditing typeshed to see places where we've had to add |
…tional argument I could not find how to do this in the documentation, but eventually saw the example in python#6580 (comment)
…tional argument I could not find how to do this in the documentation, but eventually saw the example in python#6580 (comment)
There also seems to be a related issue with passing keyword arguments with defaults from one overloaded function to another: from typing import Literal, overload
@overload
def f(*, arg: Literal[True] = ...) -> None:
...
@overload
def f(*, arg: Literal[False]) -> None:
...
def f(*, arg: bool = True) -> None:
...
@overload
def g(*, arg: Literal[True] = ...) -> None:
...
@overload
def g(*, arg: Literal[False]) -> None:
...
def g(*, arg: bool = True) -> None:
# No overload variant of "f" matches argument type "bool" [call-overload] mypy(error)
# Possible overload variants:mypy(note)
# def f(*, arg: Literal[True] = ...) -> None mypy(note)
# def f(*, arg: Literal[False]) -> None mypy(note)
return f(arg=arg) Is there a way to make this work today? |
This was fixed recently, as we decided to relax mypy's behavior w.r.t. overlapping overloads where it was technically correct but (highly) impractical. |
It appears that
Literal
s with default values lead to overlapping signatures. In the following example, the defaults are necessary since this argument follows another defaulted argument.No error
mypy 0.680+dev.4e0a1583aeb00b248e187054980771f1897a1d31 (current master)
CPython 3.7.1
The text was updated successfully, but these errors were encountered: