[ty] Prefer reflected operators by runtime class#26434
Open
charliermarsh wants to merge 1 commit into
Open
Conversation
Typing conformance resultsNo changes detected ✅Current numbersThe percentage of diagnostics emitted that were expected errors held steady at 94.47%. The percentage of expected errors that received a diagnostic held steady at 89.19%. The number of fully passing files held steady at 95/134. |
Memory usage reportSummary
Significant changesClick to expand detailed breakdownsphinx
flake8
trio
prefect
|
|
charliermarsh
force-pushed
the
charlie/reflected-operator-runtime-classes
branch
from
500bf24 to
8a7f8fa
Compare
charliermarsh
force-pushed
the
charlie/reflected-operator-runtime-classes
branch
2 times, most recently
from
4feb366 to
5def2ee
Compare
sharkdp
approved these changes
Jun 30, 2026
sharkdp
left a comment
sharkdp
left a comment
Contributor
There was a problem hiding this comment.
Thank you, this seems like an improvement.
All of this (including the pre-existing behavior) seems a bit unsound, though? 😄
In a case like this:
from typing import Literal
class Base:
def __add__(self, other: object) -> Literal["base"]:
return "base"
class Child(Base):
def __radd__(self, other: object) -> Literal["child"]:
return "child"
def f(base: Base, child: Child):
reveal_type(base + child)we reveal Literal["child"] because Child is a strict subtype of Base. But the function signature doesn't prevent me from calling f(Child(), Child()), which gives me "base" at runtime.
So instead of switching over to the __radd__ return type completely, we should probably just union it in?
| let right_is_strict_subclass = left_ty != right_ty | ||
| && match (left_ty.nominal_class(db), right_ty.nominal_class(db)) { | ||
| (Some(left_class), Some(right_class)) | ||
| if !left_ty.is_type_var() && !right_ty.is_type_var() => |
Contributor
There was a problem hiding this comment.
This is a bit unfortunate. I've seen similar checks elsewhere (in current PRs of mine, but maybe also pre-existing in the codebase). It seems like most callers of nominal_class DON'T want to resolve typevars to the nominal class of their upper bound? It would be an interesting follow up to see if we should change the implementation of that method and rather check for typevars explicitly in the places where we DO want that behavior.
In my PRs, I also needed to exclude NewType. It looks like we might also need to do that here?
Comment on lines
+248
to
+249
| The upper bound of a TypeVar is not necessarily its runtime class. The bound therefore cannot be | ||
| used to decide whether the right-hand operand's reflected method takes precedence: |
Contributor
There was a problem hiding this comment.
This test seems correct, but there is also the opposite case, I guess? When the right-hand operand type is a typevar with an upper bound of Child, then it seems like it is safe to assume that __radd__ can take precedence, since everything that the typevar could specialize to would also be a strict subclass.
Member
Author
|
Wow good call. I'm trying this in #26474. |
charliermarsh
force-pushed
the
charlie/reflected-operator-runtime-classes
branch
from
5def2ee to
6764cb1
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Python gives the right operand's reflected method priority when its runtime class is a strict subclass of the left operand's runtime class and it provides a different implementation.
Prior to #15161, this dispatch path only handled class-instance types, so checking whether the right instance type was a subtype of the left effectively modeled the runtime-class relationship. #15161 generalized the path to literals and other value types but retained the same subtype predicate. That is not equivalent for singleton types: an enum literal is not a subtype of a specific integer literal even though its runtime class can be a strict subclass of
int.This PR compares the operands' nominal runtime classes when deciding reflected-method precedence. This lets a common
IntFlagaccumulator pattern retain the enum type produced at runtime: