Skip to content

Subtype IntSet and SparseIntSet to AbstractSet{Int} - #983

Merged
oxinabox merged 3 commits into
JuliaCollections:masterfrom
JohnCobbler:fix/908-intset-abstractset
Aug 19, 2026
Merged

oxinabox merged 3 commits into
JuliaCollections:masterfrom
JohnCobbler:fix/908-intset-abstractset

Conversation

@JohnCobbler

Copy link
Copy Markdown
Contributor

Neither IntSet nor SparseIntSet subtyped AbstractSet, so generic set
algorithms and APIs that dispatch on AbstractSet ignored them.

Make both <: AbstractSet{Int} and add the helpers Base's set fallbacks need:

  • emptymutable / copymutable so filter and setdiff return the same
    concrete type instead of falling back to Set
  • filter! via unsafe_filter! (BitSet pattern) so
    intersect!(s::AbstractSet, s2::AbstractSet) works
  • delete! for SparseIntSet (was only pop!) for the filter! path
  • symdiff!(::IntSet, ::AbstractSet) and intersect!(::SparseIntSet, ::AbstractSet)
    for unambiguous dispatch

Same-type == / < / <= methods remain more specific (unchanged). Cross-type
equality with Set / between the two types becomes available via AbstractSet
fallbacks.

Docs note the AbstractSet{Int} relationship.

Fixes #908

Enable AbstractSet algorithms while keeping concrete container types.
Add emptymutable/copymutable/filter! (and SparseIntSet delete!) so
filter, setdiff, and Base's intersect!(::AbstractSet, ::AbstractSet)
stay correct. Resolve symdiff!/intersect! AbstractSet dispatch.

Fixes JuliaCollections#908
Comment thread src/int_set.jl Outdated

Base.symdiff(s::IntSet, ns) = symdiff!(copy(s), ns)
Base.symdiff!(s::IntSet, ns) = (for n in ns; symdiff!(s, n); end; s)
Base.symdiff!(s::IntSet, ns::AbstractSet) = (for n in ns; symdiff!(s, n); end; s)

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.

is this needed just to break ambiguity?

Comment thread src/sparse_int_set.jl Outdated

#Is there a more performant way to do this?
Base.intersect!(s1::SparseIntSet, ns) = copy!(s1, intersect(s1, ns))
Base.intersect!(s1::SparseIntSet, ns::AbstractSet) = copy!(s1, intersect(s1, ns))

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.

again is this needed just to break ambiguity?

Comment thread src/int_set.jl Outdated
Comment on lines +138 to +139
Base.symdiff!(s::IntSet, ns) = (for n in ns; symdiff!(s, n); end; s)
Base.symdiff!(s::IntSet, ns::AbstractSet) = (for n in ns; symdiff!(s, n); end; s)

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.

It is tempting to write this as:

Suggested change
Base.symdiff!(s::IntSet, ns) = (for n in ns; symdiff!(s, n); end; s)
Base.symdiff!(s::IntSet, ns::AbstractSet) = (for n in ns; symdiff!(s, n); end; s)
for T in (AbstractSet, Any)
Base.symdiff!(s::IntSet, ns::T) = (for n in ns; symdiff!(s, n); end; s)
end

To make it clear that they are actually identical

@JohnCobbler

Copy link
Copy Markdown
Contributor Author

Yes — those twins exist only to break the diamond with Base's AbstractSet methods once we subtype ((IntSet, AbstractSet) is more specific on the left than symdiff!(::AbstractSet, ::AbstractSet) and more specific on the right than symdiff!(::IntSet, ns)). Same pattern as BitSet.

Pushed a small follow-up that writes each pair as for T in (AbstractSet, Any) so the identical bodies are obvious (and the same for SparseIntSet intersect!).

Makes identical ambiguity-breakers explicit for IntSet symdiff! and
SparseIntSet intersect! (BitSet-style).
@oxinabox

oxinabox commented Aug 7, 2026

Copy link
Copy Markdown
Member

Can we add a comment saying that we define both to break method errors for ambiguity?

@JohnCobbler

JohnCobbler commented Aug 7, 2026 •

Copy link
Copy Markdown
Contributor Author

Done - comments on both the IntSet symdiff! and SparseIntSet intersect! twin loops.

oxinabox asked for an explicit note on both IntSet symdiff! and
SparseIntSet intersect! dual definitions.
@oxinabox
oxinabox merged commit 0ed46fb into JuliaCollections:master Aug 19, 2026
11 of 12 checks passed
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.

Intset doesn't subtype AbstractSet

2 participants