NotificationsYou must be signed in to change notification settings
Fork3.1k
Star20.1k

Fix incompatible overrides of overloaded methods in concrete subclasses#14017

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

hauntsaninja merged 7 commits intopython:masterfromhauntsaninja:generic-self

Nov 10, 2022

Merged

Fix incompatible overrides of overloaded methods in concrete subclasses#14017

hauntsaninja merged 7 commits intopython:masterfromhauntsaninja:generic-self

Nov 10, 2022

Conversation

Copy link

Collaborator

hauntsaninja commentedNov 5, 2022

Fixes#14002

hauntsaninja mentioned this pull request

Nov 5, 2022

typing: accept buffers in typing.IO.writepython/typeshed#9084

Closed

Fix incompatible overrides of overloaded methods in concrete subclasses

5c90d1e

Fixespython#14002

hauntsaninja force-pushed thegeneric-self branch fromd986d87 to5c90d1eCompare

November 5, 2022 22:34

JelleZijlstra reviewed

Nov 5, 2022

View reviewed changes

mypy/checker.py

		foriteminoriginal_type.items
		ifnotitem.arg_typesoris_subtype(active_self_type,item.arg_types[0])
		]
		iffiltered_items:

Copy link

Member

JelleZijlstraNov 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

If there are no filtered_items, shouldn't we always treat the override as compatible? e.g. in yourF test below, I think the override is actually compatible: the base class says nothing about what the method should return if self is anA[bytes].

Copy link

CollaboratorAuthor

hauntsaninjaNov 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Yeah, I wasn't sure about this, but it seemed like a weird situation so I chose to fail. Feels sketchy to introduce aCallable[..., Any] ... I'll add a comment and if it ever comes up we can reconsider

This comment has been minimized.

more tests

d62b1c7

Copy link

CollaboratorAuthor

hauntsaninja commentedNov 5, 2022

I think the mypy_primer hits are from a different bug, will fix in another PR

failing self test

8e575a5

hauntsaninja added a commit to hauntsaninja/mypy that referenced this pull request

Nov 5, 2022

Fix bug in overload compatibility checking

41733c4

Discovered as part ofpython#14017

hauntsaninja mentioned this pull request

Nov 5, 2022

Fix and optimise overload compatibility checking#14018

Merged

hauntsaninja added a commit that referenced this pull request

Nov 6, 2022

Fix and optimise overload compatibility checking (#14018)

807da26

Discovered as part of#14017

hauntsaninja added3 commits

November 5, 2022 18:53

Merge remote-tracking branch 'origin/master' into generic-self

60a5d9f

add comment

992e252

more comment

7606d50

JelleZijlstra approved these changes

Nov 6, 2022

View reviewed changes

This comment has been minimized.

Copy link

CollaboratorAuthor

hauntsaninja commentedNov 6, 2022

Hmm, there are still some issues involving type variables:

from typing import overload, TypeVar, UnionZAT = TypeVar("ZAT", bound="ZA")class ZA:    @overload    def f(self: ZAT, x: int) -> ZAT: ...    @overload    def f(self, x: str) -> None: ...    @overload    def f(self: ZAT) -> bytes: ...    def f(*a, **kw): ...class ZB(ZA):    @overload    def f(self, x: int) -> ZB: ...    @overload    def f(self, x: str) -> None: ...    def f(*a, **kw): ...

^this correctly gives an error on master, but this PR removes the error.

Given that this fixes a real issue I might add an xfail test and merge anyway, but let me see if I can figure out a change to make this work as well

add xfail test

635e575

Copy link

CollaboratorAuthor

hauntsaninja commentedNov 6, 2022

Okay I added the xfail test. The correct solution probably involves thebind_self code. I think some of the pandas errors might come back once it's fixed too

Copy link

Contributor

github-actionsbot commentedNov 6, 2022

Diff frommypy_primer, showing the effect of this PR on open source code:

pandas-stubs (https://github.com/pandas-dev/pandas-stubs)+ pandas-stubs/core/series.pyi:1752: error: Unused "type: ignore" commentpandas (https://github.com/pandas-dev/pandas)+ pandas/core/series.py:3380: error: Unused "type: ignore" comment+ pandas/core/series.py:5132: error: Unused "type: ignore" comment+ pandas/core/frame.py:5531: error: Unused "type: ignore" comment

hauntsaninja requested a review fromJelleZijlstra

November 10, 2022 23:31

Copy link

CollaboratorAuthor

hauntsaninja commentedNov 10, 2022

Let me know if you think the new false negative is not acceptable

JelleZijlstra approved these changes

Nov 10, 2022

View reviewed changes

Copy link

Member

JelleZijlstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

The xfail case seems fairly contrived and false negatives are better than false positives, so I think this is an improvement. Thanks!

hauntsaninja merged commita48dd5a intopython:master

Nov 10, 2022

hauntsaninja deleted the generic-self branch

November 10, 2022 23:37

hauntsaninja added a commit to hauntsaninja/mypy that referenced this pull request

Mar 11, 2023

Fix incompatible overrides of overloaded generics with self types

262e6e0

Fixespython#14866Basically does the potential todo I'd mentioned inpython#14017

hauntsaninja mentioned this pull request

Mar 11, 2023

Fix incompatible overrides of overloaded generics with self types#14882

Merged

JelleZijlstra pushed a commit that referenced this pull request

Mar 11, 2023

Fix incompatible overrides of overloaded generics with self types (#1…

e2e5d7f

…4882)Fixes#14866Basically does the potential todo I'd mentioned in#14017

hauntsaninja added a commit to hauntsaninja/mypy that referenced this pull request

Apr 13, 2023

Fix bounded self types in override incompatibility checking

0ac2f69

Fixespython#15041Related topython#14882 andpython#14017

hauntsaninja mentioned this pull request

Apr 13, 2023

Fix bounded self types in override incompatibility checking#15045

Merged

tyralla mentioned this pull request

Apr 14, 2023

Fix type argument inference for overloaded functions with explicit self types (Fixes #14943).#14975

Closed

hauntsaninja added a commit that referenced this pull request

Apr 18, 2023

Fix bounded self types in override incompatibility checking (#15045)

3d9661c

Fixes#15041Related to#14882 and#14017 (the older one is where I added the xfailtest)

Labels

None yet

Movatterモバイル変換

Uh oh!

Fix incompatible overrides of overloaded methods in concrete subclasses#14017

Fix incompatible overrides of overloaded methods in concrete subclasses#14017

Uh oh!

Conversation

hauntsaninja commentedNov 5, 2022

Uh oh!

JelleZijlstraNov 5, 2022

Choose a reason for hiding this comment

Uh oh!

hauntsaninjaNov 5, 2022

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

hauntsaninja commentedNov 5, 2022

Uh oh!

This comment has been minimized.

hauntsaninja commentedNov 6, 2022

Uh oh!

hauntsaninja commentedNov 6, 2022

Uh oh!

github-actionsbot commentedNov 6, 2022

Uh oh!

hauntsaninja commentedNov 10, 2022

Uh oh!

JelleZijlstra left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants