Add reflected set operators to AbstractSet - #16328
Conversation
collections.abc.Set defines __rand__, __ror__, __rsub__, and __rxor__ alongside their forward counterparts, but the stub only had __and__, __or__, __sub__, and __xor__. Without them, a class that only implements __rsub__ (etc.) won't satisfy a Protocol expecting it, even though set and frozenset both do this at runtime, since the reflected methods just fall back to the same underlying implementation. For __rand__/__ror__/__rxor__ this is exact: CPython's _collections_abc.Set assigns them as literal aliases of __and__/__or__/__xor__ (same function object), which makes sense given intersection, union, and symmetric difference don't care which operand comes first. __rsub__ has its own separate body, though, since subtraction isn't commutative. Reading it shows the result's elements come from whatever the *other* operand contributes (the elements that survive filtering against self), not from self's own element type, so its signature takes an Iterable[_T] and returns AbstractSet[_T] rather than mirroring __sub__. Verified against the reproduction from the issue (a Protocol with just __rsub__/__rand__ failing to accept a plain set()) with both mypy and pyright, and confirmed the assignment error reappears if the fix is reverted. mypy_test.py and pyright_test.py both pass on stdlib/typing.pyi across all supported Python versions, and stubtest_stdlib.py shows no new errors for typing.
This comment has been minimized.
This comment has been minimized.
Adding __rsub__ to AbstractSet in the previous commit surfaces two real Liskov substitution violations that already existed but were invisible before: multiprocessing.managers._BaseSetProxy.__rsub__ returns a plain set instead of an AbstractSet-conformant value, and boltons' IndexedSet.__rsub__ returns a distinct _RSub protocol type that isn't a Set at all. Both subclasses were already inconsistent with what Set.__rsub__ promises, this fix just makes that visible for the first time. Silenced both the same way this file already handles other set operators with the same kind of mismatch (see __ior__, __ixor__, __ror__, __rxor__ nearby), rather than trying to reshape either class's actual runtime behavior to fit the protocol.
This comment has been minimized.
This comment has been minimized.
Adding __rsub__ (and the other reflected operators) to AbstractSet meant set and frozenset now inherited it instead of relying purely on their forward methods, and mypy's subclass-priority rule for reflected binary operators started routing some calls through the inherited abstract version instead of the concrete one whenever the right operand's type was a subtype of the left operand's element type. That changed inferred types from concrete set/frozenset to plain AbstractSet in that case, which the regression test suite catches (stdlib/@tests/test_cases/builtins/check_set.py). Giving set and frozenset their own __rand__/__ror__/__rsub__/__rxor__ restores the previous concrete return types. The forward and reflected operators on these two classes are still allowed to overlap in a way mypy considers unsafe in the abstract, same as their existing __iand__/__ior__/__isub__/__ixor__ neighbors already do, so this follows that same established pattern rather than introducing a new one.
|
Diff from mypy_primer, showing the effect of this PR on open source code: prefect (https://github.com/PrefectHQ/prefect)
+ src/prefect/server/services/task_run_recorder.py:146: error: Incompatible return value type (got "set[Any]", expected "frozenset[str]") [return-value]
|
|
I'm not sure why there is a primer hit now. Considering that return (
frozenset(
column.name
for column in db.TaskRun.__table__.columns
if column.nullable
and column.default is None
and column.server_default is None
)
# the ON CONFLICT WHERE clause compares against this column, and
# `x < NULL` is NULL, so a NULL-filled row would silently skip its update
- {"state_timestamp"}
# ON CONFLICT matches rows on these columns, and NULL matches nothing, so
# a filled row would insert a duplicate instead of updating. Excluding them
# also keeps `flow_run_id` out of the coalesce, so an event with no flow
# run still clears it.
- {column.key for column in db.orm.task_run_unique_upsert_columns}
) |
|
Thanks for digging into this, and sorry for the noise. I traced it down: the returned expression is a plain set literal ( I could reproduce it in isolation: it only shows up when the set literal sits directly on the right of The prefect code itself is unaffected at runtime either way (the subtraction still returns a real |
Closes #15532
collections.abc.Setdefines__rand__,__ror__,__rsub__, and__rxor__alongside their forward counterparts, but the stub only had__and__,__or__,__sub__, and__xor__. Without them, aProtocolexpecting just the reflected method (as in the issue's example) rejectsset()/frozenset()even though both satisfy it at runtime.For
__rand__/__ror__/__rxor__this is exact: CPython's_collections_abc.Setassigns them as literal aliases of__and__/__or__/__xor__(the same function object), which tracks with intersection, union, and symmetric difference not caring which operand comes first.__rsub__has its own separate body, though, since subtraction isn't commutative:The result's elements come from whatever
othercontributes (filtered againstself), not fromself's own element type, so I typed it asother: Iterable[_T]returningAbstractSet[_T]rather than mirroring__sub__'s signature. The issue's suggested signature (matching__sub__'s shape) would have been wrong for this reason.The issue also asks about
__ror__/__rxor__but says those weren't looked into yet since they seemed more complicated. Given they're literal aliases at the runtime level, I went ahead and added them too rather than leaving that gap, since the reasoning is the same as__and__/__or__.Verified against the issue's own reproduction (a
Protocolwith just__rsub__/__rand__failing to accept a plainset()) with both mypy and pyright, and confirmed the assignment error reappears if the fix is reverted.mypy_test.pyandpyright_test.pyboth pass onstdlib/typing.pyiacross all supported Python versions, andstubtest_stdlib.pyshows no new errors fortyping(the one__rsub__-related line in its output is fordecimal.Decimal, unrelated and already there before this change).