-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Fix subtyping between ParamSpecs #15892
Fix subtyping between ParamSpecs #15892
Conversation
@@ -6483,7 +6483,7 @@ P = ParamSpec("P") | |||
R = TypeVar("R") | |||
|
|||
@overload | |||
def func(x: Callable[Concatenate[Any, P], R]) -> Callable[P, R]: ... | |||
def func(x: Callable[Concatenate[Any, P], R]) -> Callable[P, R]: ... # E: Overloaded function signatures 1 and 2 overlap with incompatible return types |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And now this error removed in my previous PR is back, LOL.
This comment has been minimized.
This comment has been minimized.
Quick comment since I haven't looked over the code yet:
My impression (I forgot everything...) is that it's mandated. But now I looked back at the PEP and found this example, which is not allowed by strict concatenate: def add(f: Callable[P, int]) -> Callable[Concatenate[str, P], None]:
def foo(s: str, *args: P.args, **kwargs: P.kwargs) -> None: # Accepted
pass
[snip]
return foo # Accepted |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic makes sense to me. Not entirely sure why you switched the default but changing that's fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I notice you use Protocol
instead of Generic
. While this should not care about that (obviously), mind adding one or shifting a test case over?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will add one test case.
Without it one test case fails. Also it is a deviation from the PEP, so probably should not be on by default. (Btw I noticed in one test case you added: "this is one noticeable deviation from PEP but I believe it is for the better", so I guess it was an intentional decision). |
Diff from mypy_primer, showing the effect of this PR on open source code: discord.py (https://github.com/Rapptz/discord.py)
+ discord/ext/commands/core.py:1799: error: Overloaded function signatures 1 and 2 overlap with incompatible return types [misc]
|
Fixes #14169
Fixes #14168
Two sings here:
strict_concatenate
check should be off by default (IIUC it is not mandated by the PEP)cc @A5rocks