Skip to content

fix cardinality of Permutations_setk #41024

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
+35 −7
Open

fix cardinality of Permutations_setk #41024

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1f8c715
fix cardinality of Permutations_setk
mantepse Oct 10, 2025
4db2e93
add minimal docstring to the cardinality methods
mantepse Oct 11, 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
42 changes: 35 additions & 7 deletions src/sage/combinat/permutation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@
import operator
from typing import TYPE_CHECKING

from sage.arith.misc import factorial, multinomial
from sage.arith.misc import factorial, multinomial, falling_factorial
from sage.categories.finite_enumerated_sets import FiniteEnumeratedSets
from sage.categories.finite_permutation_groups import FinitePermutationGroups
from sage.categories.finite_weyl_groups import FiniteWeylGroups
Expand Down Expand Up @@ -2144,7 +2144,7 @@ def ishift(self, i):
Return the ``i``-shift of ``self``. If an ``i``-shift of ``self``
can't be performed, then ``self`` is returned.
An `i`-shift can be applied when `i` is not inbetween `i-1` and
An `i`-shift can be applied when `i` is not between `i-1` and
`i+1`. The `i`-shift moves `i` to the other side, and leaves the
relative positions of `i-1` and `i+1` in place. All other entries
of the permutations are also left in place.
Expand Down Expand Up @@ -6243,7 +6243,7 @@ def __init__(self, n, k):
"""
TESTS::
sage: P = Permutations(3,2)
sage: P = Permutations(5, 3)
sage: TestSuite(P).run()
"""
self.n = ZZ(n)
Expand Down Expand Up @@ -6320,6 +6320,8 @@ def __iter__(self) -> Iterator[Permutation]:

def cardinality(self) -> Integer:
"""
Return the cardinality of the set.
EXAMPLES::
sage: Permutations(3,0).cardinality()
Expand All @@ -6334,7 +6336,7 @@ def cardinality(self) -> Integer:
0
"""
if 0 <= self._k <= self.n:
return factorial(self.n) // factorial(self.n - self._k)
return falling_factorial(self.n, self._k)
return ZZ.zero()

def random_element(self):
Expand Down Expand Up @@ -6409,7 +6411,7 @@ def __init__(self, mset):
"""
TESTS::
sage: S = Permutations(['c','a','c'])
sage: S = Permutations(['c','a','c','d'])
sage: TestSuite(S).run()
"""
self.mset = mset
Expand Down Expand Up @@ -6898,7 +6900,7 @@ def __init__(self, mset, k):
"""
TESTS::
sage: P = Permutations([1,2,2],2)
sage: P = Permutations([1,2,2,3], 2)
sage: TestSuite(P).run() # needs sage.libs.gap
"""
Permutations_mset.__init__(self, mset)
Expand Down Expand Up @@ -7014,12 +7016,26 @@ def __init__(self, s, k):
"""
TESTS::
sage: P = Permutations([1,2,4],2)
sage: P = Permutations([1,2,4,5], 2)
Copy link
Contributor

Choose a reason for hiding this comment

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

any reason for modifying the test? The test suite doesn't detect failure on the old tests, but does detect failure on the new one?

(maybe it's safer to run the test suite on both old and new object, but I don't think it's too bad either)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly, the old test is si small that it passes.

sage: TestSuite(P).run()
"""
Permutations_set.__init__(self, s)
self._k = k

def cardinality(self) -> Integer:
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this should start with "Return the cardinality of this set.", or similar.

Other than this, LGTM.

Return the cardinality of the set.
EXAMPLES::
sage: Permutations([1,2,4,5], 2).cardinality()
12
"""
n = len(self._set)
if 0 <= self._k <= n:
return falling_factorial(n, self._k)
return ZZ.zero()

def __contains__(self, x):
"""
EXAMPLES::
Expand Down Expand Up @@ -9841,6 +9857,8 @@ def __init__(self, n):

def cardinality(self):
"""
Return the cardinality of the set.
EXAMPLES::
sage: Permutations(5, avoiding=[1, 3, 2]).cardinality()
Expand Down Expand Up @@ -9918,6 +9936,8 @@ def __init__(self, n):

def cardinality(self) -> Integer:
"""
Return the cardinality of the set.
EXAMPLES::
sage: Permutations(5, avoiding=[1, 2, 3]).cardinality()
Expand Down Expand Up @@ -9990,6 +10010,8 @@ def __init__(self, n):

def cardinality(self):
"""
Return the cardinality of the set.
EXAMPLES::
sage: Permutations(5, avoiding=[3, 2, 1]).cardinality()
Expand Down Expand Up @@ -10022,6 +10044,8 @@ def __init__(self, n):

def cardinality(self):
"""
Return the cardinality of the set.
EXAMPLES::
sage: Permutations(5, avoiding=[2, 3, 1]).cardinality()
Expand Down Expand Up @@ -10054,6 +10078,8 @@ def __init__(self, n):

def cardinality(self):
"""
Return the cardinality of the set.
EXAMPLES::
sage: Permutations(5, avoiding=[3, 1, 2]).cardinality()
Expand Down Expand Up @@ -10086,6 +10112,8 @@ def __init__(self, n):

def cardinality(self):
"""
Return the cardinality of the set.
EXAMPLES::
sage: Permutations(5, avoiding=[2, 1, 3]).cardinality()
Expand Down
Loading