Add support for conditionally defined overloads #10712

cdce8p · 2021-06-25T10:55:25Z

Description

This PR allows users to define overloads conditionally, e.g., based on the Python version. At the moment this is only possible if all overloads are contained in the same block which requires duplications.

from typing import overload, Any
import sys

class A: ...
class B: ...

if sys.version_info >= (3, 9):
    class C: ...


@overload
def func(g: int) -> A: ...

@overload
def func(g: bytes) -> B: ...

if sys.version_info >= (3, 9):
    @overload
    def func(g: str) -> C: ...

def func(g: Any) -> Any: ...

Closes #9744

Test Plan

Unit tests have been added.

Limitations

Only if is supported. Support for elif and else might be added in the future. However, I believe that the single if as shown in the example is also the most common use case.

The change itself is fully backwards compatible, i.e. the current workaround (see below) will continue to function as expected.

Update: Seems like support for elif and else is required for the tests to pass.

Update: Added support for elif and else.

Current workaround

from typing import overload, Any
import sys

class A: ...
class B: ...

if sys.version_info >= (3, 9):
    class C: ...


if sys.version_info >= (3, 9):
    @overload
    def func(g: int) -> A: ...

    @overload
    def func(g: bytes) -> B: ...

    @overload
    def func(g: str) -> C: ...

    def func(g: Any) -> Any: ...

else:
    @overload
    def func(g: int) -> A: ...

    @overload
    def func(g: bytes) -> B: ...

    def func(g: Any) -> Any: ...

cdce8p · 2021-09-22T12:03:01Z

@JukkaL Maybe you would like to take a look at this, too? I believe it could be another good usability improvement, especially considering that pyright already supports it.

JukkaL

Sorry for the slow response! Left a question about elif/else blocks. Overall it seems like this could be a useful feature, especially in stubs.

JukkaL · 2021-11-12T16:53:47Z

test-data/unit/check-overloading.test

+    @overload
+    def f1(g: str) -> B: ...
+    def f1(g: Any) -> Any: ...
+reveal_type(f1(42))  # N: Revealed type is "__main__.A"


What happens in cases like this:

@overload def f(x: A) -> A: ... if x: @overload def f(x: B) -> B: ... elif y: @overload def f(x: C) -> C: ... else: @overload def f(x: D) -> D: ...

Consider all combinations of bool values for x and y. For example, if x is always false but y is always true, should we pick up C -> C from the elif block? We should have a test case that covers this. If this isn't supported, there should be a useful error message.

97littleleaf11 · 2021-11-26T02:26:20Z

Any progress on this?

cdce8p · 2021-11-26T02:31:38Z

Any progress on this?

@97littleleaf11 I hadn't had much time to look at it so far. Sorry about that. If it's urgent I can look at it sooner but from what I've seen the cutoff for the next release has already happened. I hope to address the comment next week.

97littleleaf11 · 2021-11-26T03:25:29Z

@cdce8p Oh, I was checking the status of existing PRs. Just take your time.

…oads

cdce8p · 2022-03-01T22:47:47Z

Any chance to get a review or move forward with this PR? I fully understand that it isn't the easiest with all the recursions in fastparse.py. However, I've added extensive tests (> 800 lines for 160 lines of code) and mypy_primer has also been happy. For me personally, it would help immensely to reduce code duplications and be able to delete over 300 lines of overload definitions in one case alone.

Please let me know if or how I can help.

JelleZijlstra · 2022-03-02T02:22:07Z

test-data/unit/check-functions.test

 from typing import Any
 x = None # type: Any
 if x:
+    pass  # some other node


Why is this needed?

Fixed in 3d0397a. Starting If stmts are no longer merged if they don't contain at least one overload.

mypy/fastparse.py

JelleZijlstra · 2022-03-02T02:38:42Z

test-data/unit/check-overloading.test

+def f3(g: A) -> A: ...
+@overload
+def f3(g: B) -> B: ...
+if maybe_true:  # E: Name "maybe_true" is not defined


What if maybe_true is defined but of type bool? It seems like you just silently ignore all conditionally defined overloads. It would make more sense to me to produce an error.

914d517, mypy would now emit an error for it.

Condition cannot be inferred, unable to merge overloads [misc]

docs/source/more_types.rst

github-actions · 2022-03-03T05:32:34Z

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

cdce8p · 2022-03-03T12:57:50Z

Thanks for taking the time to review my PR @JelleZijlstra!
I've been looking forward to this one for a long time.

This comment has been minimized.

Sign in to view

cdce8p marked this pull request as draft June 25, 2021 11:31

cdce8p force-pushed the conditional-overloads branch from a4fba62 to 3f102d3 Compare June 27, 2021 11:44

cdce8p marked this pull request as ready for review June 27, 2021 12:03

cdce8p force-pushed the conditional-overloads branch 2 times, most recently from a7ffa2c to d9ceadf Compare July 7, 2021 19:53

cdce8p force-pushed the conditional-overloads branch 3 times, most recently from 151393b to 50dfa0d Compare August 2, 2021 07:13

cdce8p mentioned this pull request Aug 2, 2021

@overload cannot be used with conditional "if TYPE_CHECKING" #9744

Closed

cdce8p force-pushed the conditional-overloads branch from 50dfa0d to 425a8d4 Compare September 13, 2021 15:21

cdce8p force-pushed the conditional-overloads branch from 425a8d4 to 23439c6 Compare October 13, 2021 21:49

cdce8p force-pushed the conditional-overloads branch from 23439c6 to 1e914d8 Compare October 26, 2021 10:31

cdce8p force-pushed the conditional-overloads branch from 1e914d8 to 301072d Compare November 7, 2021 12:33

cdce8p added 2 commits November 11, 2021 19:56

Add support for conditionally defined overloads

99436ee

Bugfix

7e7502b

cdce8p force-pushed the conditional-overloads branch from 301072d to 7e7502b Compare November 11, 2021 18:56

JukkaL reviewed Nov 12, 2021

View reviewed changes

cdce8p added 7 commits December 14, 2021 14:44

Merge remote-tracking branch 'upstream/master' into conditional-overl…

52ba893

…oads

Redo logic to support elif + else

3effa15

Fix typing issues

ee6ad3c

Fix small issue with merging IfStmt

bc486e0

Update existing tests

a1370e0

Add additional tests

0d2dee3

Fix check-functions tests

ae394cc

This comment has been minimized.

Sign in to view

Merge remote-tracking branch 'upstream/master' into conditional-overl…

7cb5eac

…oads

This comment has been minimized.

Sign in to view

JelleZijlstra self-requested a review March 1, 2022 22:58

JelleZijlstra requested changes Mar 2, 2022

View reviewed changes

cdce8p added 4 commits March 2, 2022 11:57

Apply suggestions from review

80b05d5

Don't merge starting If blocks without overloads

3d0397a

Emit error if condition can't be inferred

914d517

Add additional test cases

4f79d43

JelleZijlstra self-requested a review March 2, 2022 20:21

This comment has been minimized.

Sign in to view

Add documentation

4776070

cdce8p force-pushed the conditional-overloads branch from f529270 to 4776070 Compare March 2, 2022 21:25

This comment has been minimized.

Sign in to view

JelleZijlstra reviewed Mar 3, 2022

View reviewed changes

Copyedits to the docs

cd1e9b0

JelleZijlstra approved these changes Mar 3, 2022

View reviewed changes

JelleZijlstra self-assigned this Mar 3, 2022

JelleZijlstra merged commit 0777c10 into python:master Mar 3, 2022

cdce8p deleted the conditional-overloads branch March 3, 2022 09:58

cdce8p mentioned this pull request Mar 3, 2022

Small doc improvements for conditional overloads #12283

Merged

cdce8p mentioned this pull request Mar 12, 2022

Add conditional overloads [TreeRebuilder] pylint-dev/astroid#1470

Merged

This was referenced Mar 23, 2022

Use conditional overloads to simplify several stdlib functions python/typeshed#7540

Merged

Enforce error codes on type: ignores #10718

Closed

cdce8p mentioned this pull request Apr 17, 2022

Fix nested overload merging #12607

Merged

DetachHead mentioned this pull request Aug 5, 2022

support kw_only on dataclasses pydantic/pydantic#3674

Merged

5 tasks

Uh oh!

Add support for conditionally defined overloads #10712

Add support for conditionally defined overloads #10712

Uh oh!

Conversation

cdce8p commented Jun 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Test Plan

Limitations

Current workaround

Uh oh!

This comment has been minimized.

cdce8p commented Sep 22, 2021

Uh oh!

JukkaL left a comment

Choose a reason for hiding this comment

Uh oh!

JukkaL Nov 12, 2021

Choose a reason for hiding this comment

Uh oh!

97littleleaf11 commented Nov 26, 2021

Uh oh!

cdce8p commented Nov 26, 2021

Uh oh!

97littleleaf11 commented Nov 26, 2021

Uh oh!

This comment has been minimized.

This comment has been minimized.

cdce8p commented Mar 1, 2022

Uh oh!

JelleZijlstra Mar 2, 2022

Choose a reason for hiding this comment

Uh oh!

cdce8p Mar 2, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

JelleZijlstra Mar 2, 2022

Choose a reason for hiding this comment

Uh oh!

cdce8p Mar 2, 2022

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Mar 3, 2022

Uh oh!

cdce8p commented Mar 3, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cdce8p commented Jun 25, 2021 •

edited

Loading