Repository navigation
SetManager inherits an API surface it doesn't want; decide its real interface #243
Description
Summary
SetManager subclasses collections.abc.Set, and most of its sharp edges trace back to that choice. 1.3.0 made the inherited surface safe (#238, PR #240); 2.0 should decide whether it should exist.
Status after the 1.3.0 release: the parts common to both options already shipped as deprecations — __call__ and missing-member remove() emit DeprecationWarning, and discard() exists as the ignore-missing replacement (#253). The remaining decision here is only the option-A-vs-B question (the operator surface and the Set ABC), plus executing the removals.
Symptoms, all one root cause
- Wrong ABC.
collections.abc.Setis the immutable set interface, yet the class exposesadd()/remove()/clear(). The honest ABC for the current behavior isMutableSet— but see below for whether we want the ABC at all. remove()hasdiscard()semantics. It silently ignores missing members, the opposite of whatset.removepromises.CONSTANTS.titles.remove('typo-entry')reports nothing. (1.3.0: now emitsDeprecationWarning;discard()added.)__call__is an escape hatch.CONSTANTS.titles()returns the raw underlyingset, allowing mutation that bypasses both normalization and the_on_changecache-invalidation hook thatConstantsrelies on. (1.3.0: now emitsDeprecationWarning.)- The operator surface exists only by inheritance. The
Setmixins inject|/&/-/^operators that accept arbitrary iterables and construct results outside the class's normalization invariant. Making them safe took six guarded, operand-normalizing overrides with ~10type: ignorecomments (PR Fix bare string to SetManager silently shredding into characters #240) — when you fight both typeshed and the runtime ABC to guard methods you never designed, the supertype is wrong.
Related: the __contains__ normalization asymmetry is now tracked separately as #244 (1.3.0); whichever option lands here, membership survives unchanged.
Options
A. Stop subclassing Set; compose over a plain set (recommended).
Expose exactly what the config use case needs: add, remove, discard, clear, in, iteration, len. The one internal set-algebra use, the suffixes_prefixes_titles union, works on the underlying sets directly.
Pros:
- Smallest honest surface — every method exists on purpose, and all mutation is forced through
add()/remove(), so normalization and the_on_changecache-invalidation hook can never be bypassed - Deletes the six operator overrides and their ~10
type: ignorecomments outright; nothing inherited is left to audit when Python's ABC mixins evolve - No future symptom 5: the class can't grow new behavior without someone deliberately adding it
Cons:
isinstance(x, collections.abc.Set)stops passing; any downstream duck-typed set algebra on config sets breaks|/&/-/^on config sets stop working (mitigation: the operators only became safe in 1.3.0, so correct usage in the wild is necessarily rare)- Loses the ABC's free set-comparison semantics (
==,<=between managers); a deliberate two-line__eq__can be added back if wanted
B. Commit to the surface properly: switch to MutableSet.
Keep the normalized operators from PR #240, give remove() real KeyError semantics (discard() already shipped in 1.3.0), drop __call__ (already deprecated).
Pros:
- Familiar set idioms keep working, including
isinstancechecks MutableSet's in-place mixins are a real fit:__ior__routes each element throughadd(), sotitles |= [...]would normalize and fire_on_changefor free — the inherited behavior finally aligns with the class invariant- Least migration for anyone using operators today
Cons:
- Keeps the root cause: a large inherited surface where every current and future mixin must be audited against the normalization invariant —
MutableSetadds more, not less (pop,__iand__,__ixor__,__isub__, each of which mutates and must be verified to fire_on_change) - The typeshed-vs-runtime annotation friction from PR Fix bare string to SetManager silently shredding into characters #240 remains and grows
- Same breaking changes to
remove()/__call__anyway, so B pays most of A's migration cost while keeping the maintenance tax
Option A is preferred: no parser functionality needs set algebra on config sets, and every symptom above came from surface nobody designed. B's strongest point — __ior__ normalizing for free — fixes one operator by accident of mixin implementation, which is the same coupling that produced the bugs.
Breaking changes (hence 2.0)
isinstance(x, collections.abc.Set)checks stop passing (option A)|/&/-/^on config sets stop working (option A) or keep 1.3.0 behavior (option B)remove()of a missing member raises (discard(), shipped in 1.3.0, is the ignore-missing spelling)CONSTANTS.titles()call syntax removed — iterate or copy instead
Activity
Reconcile test_constants against the shim (buckets A, B, C)
docs: rewrite the 2.0 release log new-API-first and complete the audit
derek73 commentedon Jul 28, 2026
Shipped in 2.0.0. Delivery and release-log coverage were verified issue-by-issue in the pre-release milestone audit.