Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Fix Type Comparison Edge Cases#20277

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

Open
George-Ogden wants to merge12 commits intopython:master
base:master
Choose a base branch
Loading
fromGeorge-Ogden:type-comparison

Conversation

@George-Ogden
Copy link
Contributor

Fixes#20275 and#20041
This code snippet fails to type-check before this PR:

class X:    y: int    def __eq__(self, other: object) -> bool:        return type(other) is type(self) and other.y == self.yidx: int | float = 0if type(idx) == int == int:    x = [100][idx]

But type-checks afterwards.
This is done by finding a least type as the upper bound when the equality succeeds.

@github-actions

This comment has been minimized.

@George-Ogden
Copy link
ContributorAuthor

I don't quite understand what I should do withget_proper_type, but the mypy_primer diff seems to reduce the number of type errors.

Copy link
Collaborator

@A5rocksA5rocks left a comment

Choose a reason for hiding this comment

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

I'm not familiar with this part of mypy (this is the first time I sawTypeRange!), so my comments may be wrong. Initial pass through though.

I'm not sure about the bigger picture here, it feels somewhat hacky to rely ontype(x) == Y logic to handletype(x) == type(y). I don't have any better ideas though.

@github-actions

This comment has been minimized.

@tyralla
Copy link
Collaborator

Thanks for the PR!

Also, just a short remark/question without any deep knowledge of the matter: If I understand#20275 correctly, callingreveal_type already helps. Doesn't this mean the required narrowing logic is already available and only needs to be activated somehow for (normal) cases wherereveal_type is not used?

@George-Ogden
Copy link
ContributorAuthor

@tyralla:
before this PR, this worked best with direct comparison to types:type(x) == int => x: int (1)
this also includesy: type[int]; type(x) == y => x: int (2)
The previous logic looked at whentype() was used to separate what was narrowed and what was constant
However, it fails when comparingtype(x) == type(y) (3)
Whenreveal_type is used, it's equivalent to the second case.
after PR analyses the third case is analysed so that it looks more like
x: X; y: Y; type(x) == Y and type(y) == X (4)
But this 4th case is all-or-nothing, so either both variables have the type of the meet or they retain the originals.

Let me know if this makes it clear or if any of that doesn't make sense.

@A5rocks
Copy link
Collaborator

A5rocks commentedNov 22, 2025•
edited
Loading

Huh, then couldn't we get rid of any special casing for looking for specificallytype(x)? i.e. couldn't we just use case 1/2 always?

I imagine it's a bit strange (e.g.type(x) == int == str should like, be marked unreachable not beint & str) but maybe we just need to check for anything where theTypeRange.is_upper_bound isFalse and try using that, otherwise use themeet? I haven't really thought much about this so it's possible there's some flaw here. But this would mean e.g. for functionf(x: T) -> T we would supportf(type(x)) == int -- kind of silly, but maybe useful?

(I guess the flaw is actually that we need to know the relevant name to know what to narrow. Maybe that's why we don't do that)

@tyralla
Copy link
Collaborator

I never really thought about type narrowing withis (except for handlingNone, of course).

If I look at this example:

classX:x=1classY:y=1classZ(X,Y): ...z=Z()x:X=zy:Y=zifxisy:x.y+y.x# error: "X" has no attribute "y"# error: "Y" has no attribute "x

I ask myself if Mypy should not handle it like:

ifisinstance(x,type(y))andisinstance(y,type(x)):x.y+y.x

Which could eventually easily be implemented with the help ofTypeChecker.intersect_instances or something similar?

@George-Ogden
Copy link
ContributorAuthor

I never really thought about type narrowing withis (except for handlingNone, of course).

If I look at this example:

classX:x=1classY:y=1classZ(X,Y): ...z=Z()x:X=zy:Y=zifxisy:x.y+y.x# error: "X" has no attribute "y"# error: "Y" has no attribute "x

I ask myself if Mypy should not handle it like:

ifisinstance(x,type(y))andisinstance(y,type(x)):x.y+y.x

Which could eventually easily be implemented with the help ofTypeChecker.intersect_instances or something similar?

I had a look at this and I've modified the implementation to allow this kind of checking. It just needs a bit of cleaning up.

@George-Ogden
Copy link
ContributorAuthor

This cannot be directly handled here because== andis are handled together.
However, we can compare types instead

classX:x=1classY:y=1classZ(X,Y): ...z=Z()x:X=zy:Y=ziftype(x)istype(y):# instead of x is yx.y+y.x

@George-Ogden
Copy link
ContributorAuthor

George-Ogden commentedNov 23, 2025•
edited
Loading

The only part I'm not sure about is how to handle the error messages.

@George-Ogden
Copy link
ContributorAuthor

George-Ogden commentedNov 23, 2025•
edited
Loading

I'm also not sure about the failures - should they be flagged?

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

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

pandas (https://github.com/pandas-dev/pandas)+ pandas/core/arrays/base.py:1502: error: Redundant cast to "ExtensionArray"  [redundant-cast]ibis (https://github.com/ibis-project/ibis)- ibis/common/patterns.py:658: error: "Pattern" has no attribute "type"  [attr-defined]- ibis/common/patterns.py:934: error: "Pattern" has no attribute "patterns"  [attr-defined]jax (https://github.com/google/jax)+ jax/_src/interpreters/partial_eval.py:96: error: Unused "type: ignore" comment  [unused-ignore]+ jax/_src/interpreters/partial_eval.py:99: error: No overload variant of "get" of "dict" matches argument types "int | DArray | Tracer | Var | DBIdx | InDBIdx | OutDBIdx", "int | DArray | Tracer | Var | DBIdx | InDBIdx | OutDBIdx"  [call-overload]+ jax/_src/interpreters/partial_eval.py:99: note: Possible overload variants:+ jax/_src/interpreters/partial_eval.py:99: note:     def get(self, Name, None = ..., /) -> DBIdx | None+ jax/_src/interpreters/partial_eval.py:99: note:     def get(self, Name, DBIdx, /) -> DBIdx+ jax/_src/interpreters/partial_eval.py:99: note:     def [_T] get(self, Name, _T, /) -> DBIdx | _T+ jax/_src/interpreters/batching.py:231: error: Unused "type: ignore" comment  [unused-ignore]+ jax/_src/interpreters/batching.py:232: error: Unused "type: ignore" comment  [unused-ignore]+ jax/_src/interpreters/batching.py:234: error: No overload variant of "get" of "dict" matches argument types "int | DArray | Tracer | Var | DBIdx | InDBIdx | OutDBIdx", "int | DArray | Tracer | Var | DBIdx | InDBIdx | OutDBIdx"  [call-overload]+ jax/_src/interpreters/batching.py:234: note: Possible overload variants:+ jax/_src/interpreters/batching.py:234: note:     def get(self, Name, None = ..., /) -> DBIdx | None+ jax/_src/interpreters/batching.py:234: note:     def get(self, Name, DBIdx, /) -> DBIdx+ jax/_src/interpreters/batching.py:234: note:     def [_T] get(self, Name, _T, /) -> DBIdx | _T+ jax/_src/interpreters/batching.py:258: error: Unused "type: ignore" comment  [unused-ignore]rotki (https://github.com/rotki/rotki)+ rotkehlchen/history/events/structures/base.py:161: error: Unused "type: ignore" comment  [unused-ignore]+ rotkehlchen/history/events/structures/base.py:162: error: Unused "type: ignore" comment  [unused-ignore]+ rotkehlchen/history/events/structures/base.py:163: error: Unused "type: ignore" comment  [unused-ignore]+ rotkehlchen/history/events/structures/base.py:164: error: Unused "type: ignore" comment  [unused-ignore]+ rotkehlchen/history/events/structures/base.py:165: error: Unused "type: ignore" comment  [unused-ignore]+ rotkehlchen/history/events/structures/base.py:166: error: Unused "type: ignore" comment  [unused-ignore]+ rotkehlchen/history/events/structures/base.py:167: error: Unused "type: ignore" comment  [unused-ignore]+ rotkehlchen/history/events/structures/base.py:168: error: Unused "type: ignore" comment  [unused-ignore]+ rotkehlchen/history/events/structures/base.py:169: error: Unused "type: ignore" comment  [unused-ignore]+ rotkehlchen/history/events/structures/base.py:170: error: Unused "type: ignore" comment  [unused-ignore]+ rotkehlchen/history/events/structures/base.py:171: error: Unused "type: ignore" comment  [unused-ignore]+ rotkehlchen/history/events/structures/base.py:172: error: Unused "type: ignore" comment  [unused-ignore]pip (https://github.com/pypa/pip)+ src/pip/_internal/utils/misc.py:551: error: Redundant cast to "HiddenText"  [redundant-cast]

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

1 more reviewer

@A5rocksA5rocksA5rocks left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Usingtype(other) is type(self) only works when revealing types

3 participants

@George-Ogden@tyralla@A5rocks
[8]ページ先頭
©2009-2025 Movatter.jp