-
-
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
Make overloads respect keyword-only args #4912
Make overloads respect keyword-only args #4912
Conversation
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.
A general comment, I would rather recommend spending some time in writing a strict specification of how overloads should work, otherwise these PRs feel a bit unsystematic. Also unfortunately I think we will not be able to review this before 0.600 is released.
from foo import * | ||
[file foo.pyi] | ||
from typing import overload, TypeVar | ||
AnyStr = TypeVar('AnyStr', bytes, str) | ||
|
||
@overload | ||
def f(x: AnyStr) -> None: pass # E: Overloaded function signatures 1 and 2 overlap with incompatible return types | ||
def f(x: AnyStr) -> None: pass |
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.
This example is not very useful, I would rather check what happens for (AnyStr) -> AnyStr
.
@ilevkivskyi -- I updated the test, fyi.
I think my strategy was to stick to making changes that improve the status quo, avoid making drastic changes, and steadily iterate before trying to nail down a specification. (E.g. regardless of how we handle overloads, it'd be nice if they understood keyword-only arguments; partially removing the erasure thing was just a nice side-effect that also improves the status quo). That said, I don't mind trying to write a specification and pushing for more systemic changes if you + the mypy team think it's a good idea -- I just wasn't sure if anybody had the bandwidth to discuss/review any proposed changes. But if you think I should go for it anyways, let me know and I can give it a shot :) |
Specifying how exactly overloads should work is a long standing issue, I was going to work on this myself anyway, also I think other will agree it is an important thing to fix before PEP 484 is no more provisional, see python/typing#253 Concerning the bandwidth, discussing the specification will take less energy (at least for me) than reviewing code without specification. Also I think @JukkaL mentioned some time ago that specification and implementation should be developed in parallel, so that we don't specify something that is hard to implement. In this view you can use your PRs to test various aspects of the future proposed spec. |
ab76984
to
75dae80
Compare
This commit resolves python#1907. Specifically, it turned out that support for non-positional args in overload was never implemented to begin with. Thankfully, it also turned out the bulk of the logic we wanted was already implemented within `mypy.subtypes.is_callable_subtype`. Rather then re-implementing that code, this commit refactors that method to support any kind of check, instead of specifically subtype checks. This, as a side-effect, ended up making some partial progress towards python#4159 -- this is because unlike the existing checks, `mypy.subtypes.is_callable_subtype` *doesn't* erase types and has better support for typevars in general. The reason this commit does not fully remove type erasure from overload checks is because the new implementation still calls `mypy.meet.is_overlapping_types` which *does* perform erasure. But fixing that seemed out-of-scope for this commit, so I stopped here.
75dae80
to
c8d4c2d
Compare
Superseded by #5084. |
This pull request resolves #1907.
Specifically, it turned out that support for non-positional args in overload was never implemented to begin with. Thankfully, it also turned out the bulk of the logic we wanted was already implemented within
mypy.subtypes.is_callable_subtype
. Rather then re-implementing that code, this pull request refactors that function to support any kind of check instead of specifically subtype checks.This, as a side-effect, ended up making some partial progress towards #4159 -- this is because unlike the existing checks,
mypy.subtypes.is_callable_subtype
doesn't erase types and has better support for typevars in general.The reason this commit doesn't fully remove type erasure from overload checks is because the new implementation still calls
mypy.meet.is_overlapping_types
which does perform erasure. But fixing that seemed out-of-scope for this commit, so I stopped here.