Skip to content

Implement isdisjoint for finite sets #41107

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Implement isdisjoint for finite sets #41107

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3777f3f
Add `Set` methods
whoami730 Nov 1, 2025
f9f140f
Merge branch 'develop' into isdisjoint
whoami730 Nov 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 37 additions & 10 deletions src/sage/sets/set.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,21 +35,18 @@
# https://www.gnu.org/licenses/
# ****************************************************************************

import sage.rings.infinity
from sage.categories.enumerated_sets import EnumeratedSets
from sage.categories.finite_enumerated_sets import FiniteEnumeratedSets
from sage.categories.sets_cat import Sets
from sage.misc.cachefunc import cached_method
from sage.misc.classcall_metaclass import ClasscallMetaclass
from sage.misc.latex import latex
from sage.misc.prandom import choice
from sage.misc.cachefunc import cached_method

from sage.structure.category_object import CategoryObject
from sage.structure.element import Element
from sage.structure.parent import Parent, Set_generic
from sage.structure.richcmp import richcmp_method, richcmp, rich_to_bool
from sage.misc.classcall_metaclass import ClasscallMetaclass

from sage.categories.sets_cat import Sets
from sage.categories.enumerated_sets import EnumeratedSets
from sage.categories.finite_enumerated_sets import FiniteEnumeratedSets

import sage.rings.infinity
from sage.structure.richcmp import rich_to_bool, richcmp, richcmp_method


def has_finite_length(obj) -> bool:
Expand Down Expand Up @@ -1085,6 +1082,32 @@ def __richcmp__(self, other, op):
return rich_to_bool(op, 0)
return rich_to_bool(op, -1)

def isdisjoint(self, other):
Copy link
Member

@yyyyx4 yyyyx4 Oct 31, 2025

Choose a reason for hiding this comment

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

The standard in Sage is to add underscores between separate words in method names.

Suggested change
def isdisjoint(self, other):
def is_disjoint(self, other):

Copy link
Contributor

@mantepse mantepse Oct 31, 2025

Choose a reason for hiding this comment

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

I would suggest not to do this in this very special case, because Set is supposed to be a "better" set (it is not, in my opinion, but it tries), and it is set.isdisjoint rather than set.is_disjoint.

Copy link
Member

@yyyyx4 yyyyx4 Oct 31, 2025

Choose a reason for hiding this comment

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

Alright, I see.

Copy link
Contributor

@mantepse mantepse Nov 1, 2025

Choose a reason for hiding this comment

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

I'm afraid I have to take this back, the situation in sage is already chaotic :-(

Set (and FrozenBitset) indeed implements issubset etc., but there are some classes (MutablePoset, ManifoldSubset, RealSet, I couldn't find any others) that implement is_subset. In #41113, another class is going to implement is_subset.

It is not clear to me what's best.

Copy link
Member

@yyyyx4 yyyyx4 Nov 1, 2025

Choose a reason for hiding this comment

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

I suppose fundamentally we should make a choice and then rename/deprecate. For the time being, implementing both could be alright (it's just an extra line isdisjoint = is_disjoint in the class).

"""
Return whether ``self`` and ``other`` are disjoint.

INPUT:

- ``other`` -- a finite Set

EXAMPLES::

sage: X = Set([1,2,3])
sage: Y = Set([2,4,6])
sage: Z = Set([4,5,6])
sage: X.isdisjoint(Y)
False
sage: X.isdisjoint(Z)
True
sage: Y.isdisjoint(Z)
False
"""
if not isinstance(other, Set_object_enumerated):
raise NotImplementedError
return self.set().isdisjoint(other.set())
Comment on lines +1105 to +1107
Copy link
Contributor

@mantepse mantepse Nov 11, 2025

Choose a reason for hiding this comment

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

I think we should be much less restrictive here. For example,

sage: s = set([1,2,3])
sage: s.is_disjoint(range(4,6))
False

just works, and I see no good reason why it shouldn't. So possibly simply (untested!):

Suggested change
if not isinstance(other, Set_object_enumerated):
raise NotImplementedError
return self.set().isdisjoint(other.set())
return self.set().isdisjoint(other)


is_disjoint = isdisjoint

def issubset(self, other):
r"""
Return whether ``self`` is a subset of ``other``.
Expand Down Expand Up @@ -1113,6 +1136,8 @@ def issubset(self, other):
raise NotImplementedError
return self.set().issubset(other.set())

is_subset = issubset

def issuperset(self, other):
r"""
Return whether ``self`` is a superset of ``other``.
Expand Down Expand Up @@ -1141,6 +1166,8 @@ def issuperset(self, other):
raise NotImplementedError
return self.set().issuperset(other.set())

is_superset = issuperset

def union(self, other):
"""
Return the union of ``self`` and ``other``.
Expand Down
Loading