Skip to content

Improve ordering comparisons in operator - #16812

Open
carljm wants to merge 3 commits into
python:mainfrom
carljm:cjm/operator-rich-comparisons
Open

carljm wants to merge 3 commits into
python:mainfrom
carljm:cjm/operator-rich-comparisons

Conversation

@carljm

@carljm carljm commented Nov 15, 2026

Copy link
Copy Markdown
Member

Since #16014, operator.lt(x, y) can fail type checking even when x < y is valid, when x.__lt__(y) returns an object with no __bool__ method. Python allows x < y to return that object directly, and operator.lt(x, y) has the same behavior.

Also, the current stubs for operator.lt, le, gt, and ge require both operands to define an ordering method. Python only requires one operand to handle the comparison.

Update operator.lt, le, gt, and ge to accept ordering methods with arbitrary return types and allow heterogeneous operands where either side implements the comparison.

Assisted by Codex.

@carljm
carljm marked this pull request as draft November 15, 2026 05:38
@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

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

pandera (https://github.com/pandera-dev/pandera)
+ pandera/backends/xarray/builtin_checks.py:404: error: Unused "type: ignore" comment  [unused-ignore]
+ pandera/backends/pandas/builtin_checks.py:195: error: Unused "type: ignore" comment  [unused-ignore]

pandas (https://github.com/pandas-dev/pandas)
+ pandas/io/formats/style.py:4410: error: Unused "type: ignore" comment  [unused-ignore]
+ pandas/io/formats/style.py:4421: error: Unused "type: ignore" comment  [unused-ignore]

@carljm
carljm marked this pull request as ready for review November 15, 2026 05:55
Comment thread stdlib/_operator.pyi
_P = ParamSpec("_P")

_SupportsComparison: TypeAlias = SupportsDunderLE[Any] | SupportsDunderGE[Any] | SupportsDunderGT[Any] | SupportsDunderLT[Any]
# operator returns the rich comparison result without testing its truth value.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are you trying to say "and this is why we define these protocols here rather than using the ones in _typeshed"? If so, you should say that explicitly in this comment; a casual reader of this stub won't necessarily know that there even are similar protocols defined in _typeshed

Comment thread stdlib/_operator.pyi
# ordering methods are accepted. For example, if x.__lt__ has separate int and
# str overloads and y is int | str, neither overload accepts the whole union.
# The fallback accepts lt(x, y) because int and str each define an ordering method.
_ComparisonOperand: TypeAlias = _T | _SupportsComparison[Any]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I know that ty supports type aliases to bare typevars, but I'm not totally sure they're supported by all other type checkers; I think it would also be reasonable for a type checker to reject such a type alias as invalid. Have you double-checked that all major type checkers correctly infer this as a generic type alias?

Comment thread stdlib/_operator.pyi
def le(a: _ComparisonOperand[_T], b: _SupportsComparison[_T], /) -> Any: ...

# Equality accepts any operands. Using Any also lets mypy infer a callable
# for a dispatch table containing both equality and ordering functions.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I have no idea what "dispatch table" is supposed to refer to here

Comment thread stdlib/_operator.pyi
class _SupportsDunderGE(Protocol[_T_contra]):
def __ge__(self, other: _T_contra, /) -> object: ...

# Some projects use _SupportsComparison without a type argument.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't understand what this comment is trying to say

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants