From e057b27343f491fff228f840c04e7624e9a1bf03 Mon Sep 17 00:00:00 2001 From: David Einstein Date: Wed, 21 Jun 2023 23:00:06 -0400 Subject: [PATCH 1/6] Mind the GAP The generators for the classes Tuples and UnorderedTuples used the GAP for generating Tuples. This could be done much more simply using itertools. Also fixed a bug where if the underlying list had multiple identical elements then redundant tuples were generated. --- src/sage/combinat/tuple.py | 39 ++++++++++++++------------------------ 1 file changed, 14 insertions(+), 25 deletions(-) diff --git a/src/sage/combinat/tuple.py b/src/sage/combinat/tuple.py index 7e7f85c61ef..254ce7f5ff4 100644 --- a/src/sage/combinat/tuple.py +++ b/src/sage/combinat/tuple.py @@ -16,12 +16,13 @@ # https://www.gnu.org/licenses/ # **************************************************************************** -from sage.libs.gap.libgap import libgap +#from sage.libs.gap.libgap import libgap +from sage.arith.misc import binomial from sage.rings.integer_ring import ZZ from sage.structure.parent import Parent from sage.structure.unique_representation import UniqueRepresentation from sage.categories.finite_enumerated_sets import FiniteEnumeratedSets - +from itertools import product, combinations_with_replacement class Tuples(Parent, UniqueRepresentation): """ @@ -75,7 +76,7 @@ def __init__(self, S, k): """ self.S = S self.k = k - self._index_list = [S.index(s) for s in S] + self._index_list = list(dict.fromkeys(S.index(s) for s in S)) category = FiniteEnumeratedSets() Parent.__init__(self, category=category) @@ -105,22 +106,9 @@ def __iter__(self): ['i', 'i'], ['n', 'i'], ['s', 'n'], ['t', 'n'], ['e', 'n'], ['i', 'n'], ['n', 'n']] """ - S = self.S - k = self.k - import copy - if k <= 0: - yield [] - return - if k == 1: - for x in S: - yield [x] - return - - for s in S: - for x in Tuples(S, k - 1): - y = copy.copy(x) - y.append(s) - yield y + + for p in product(self._index_list, repeat=self.k): + yield [self.S[i] for i in reversed(p)] def cardinality(self): """ @@ -133,7 +121,7 @@ def cardinality(self): sage: Tuples(S,2).cardinality() 25 """ - return ZZ(libgap.NrTuples(self._index_list, ZZ(self.k))) + return ZZ(len(self._index_list)).__pow__(self.k) Tuples_sk = Tuples @@ -178,7 +166,7 @@ def __init__(self, S, k): """ self.S = S self.k = k - self._index_list = [S.index(s) for s in S] + self._index_list = list(dict.fromkeys(S.index(s) for s in S)) category = FiniteEnumeratedSets() Parent.__init__(self, category=category) @@ -191,7 +179,7 @@ def __repr__(self): """ return "Unordered tuples of %s of length %s" % (self.S, self.k) - def list(self): + def __iter__(self): """ EXAMPLES:: @@ -202,8 +190,9 @@ def list(self): [['a', 'a'], ['a', 'b'], ['a', 'c'], ['b', 'b'], ['b', 'c'], ['c', 'c']] """ - ans = libgap.UnorderedTuples(self._index_list, ZZ(self.k)) - return [[self.S[i] for i in l] for l in ans] + for ans in combinations_with_replacement(self._index_list, self.k): + yield [self.S[i] for i in ans] + def cardinality(self): """ @@ -213,7 +202,7 @@ def cardinality(self): sage: UnorderedTuples(S,2).cardinality() 15 """ - return ZZ(libgap.NrUnorderedTuples(self._index_list, ZZ(self.k))) + return binomial(len(self._index_list) + self.k - 1, self.k) UnorderedTuples_sk = UnorderedTuples From aba3954b02055c4019e308d17eee1e1e1971b73b Mon Sep 17 00:00:00 2001 From: David Einstein Date: Thu, 22 Jun 2023 17:40:14 -0400 Subject: [PATCH 2/6] Finish decoupling tuple.py from GAP. This commit is just a cosmetic change from the last (deleting a commented out line). This should finish bug #35784. --- src/sage/combinat/tuple.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/sage/combinat/tuple.py b/src/sage/combinat/tuple.py index 254ce7f5ff4..543160597f0 100644 --- a/src/sage/combinat/tuple.py +++ b/src/sage/combinat/tuple.py @@ -16,7 +16,6 @@ # https://www.gnu.org/licenses/ # **************************************************************************** -#from sage.libs.gap.libgap import libgap from sage.arith.misc import binomial from sage.rings.integer_ring import ZZ from sage.structure.parent import Parent From 5702830e6b2cb3071385e9590a560aaeadc6a5e3 Mon Sep 17 00:00:00 2001 From: David Einstein Date: Thu, 22 Jun 2023 22:06:12 -0400 Subject: [PATCH 3/6] Added testcases to Tuples for undelying lists with duplicates Also removed a blank line that irritated the linter. --- src/sage/combinat/tuple.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/sage/combinat/tuple.py b/src/sage/combinat/tuple.py index 543160597f0..9002e5d7fb3 100644 --- a/src/sage/combinat/tuple.py +++ b/src/sage/combinat/tuple.py @@ -104,8 +104,10 @@ def __iter__(self): ['n', 'e'], ['s', 'i'], ['t', 'i'], ['e', 'i'], ['i', 'i'], ['n', 'i'], ['s', 'n'], ['t', 'n'], ['e', 'n'], ['i', 'n'], ['n', 'n']] + sage: Tuples([1,1,2],3).list() + [[1, 1, 1], [2, 1, 1], [1, 2, 1], [2, 2, 1], [1, 1, 2], + [2, 1, 2], [1, 2, 2], [2, 2, 2]] """ - for p in product(self._index_list, repeat=self.k): yield [self.S[i] for i in reversed(p)] @@ -188,11 +190,12 @@ def __iter__(self): sage: UnorderedTuples(["a","b","c"],2).list() [['a', 'a'], ['a', 'b'], ['a', 'c'], ['b', 'b'], ['b', 'c'], ['c', 'c']] + sage: UnorderedTuples([1,1,2],3).list() + [[1, 1, 1], [1, 1, 2], [1, 2, 2], [2, 2, 2]] """ for ans in combinations_with_replacement(self._index_list, self.k): yield [self.S[i] for i in ans] - def cardinality(self): """ EXAMPLES:: From dffe683fbc823507ef46f4a9c03e7ba4a79c4dfd Mon Sep 17 00:00:00 2001 From: David Einstein Date: Sun, 25 Jun 2023 09:15:26 -0400 Subject: [PATCH 4/6] Have tuple.py return tuples instead of lists This has Tuples and UnorderedTuples return tuples. Need to fix up schemes/projective/projective_space.py as it assumes the tuples are lists. --- src/sage/combinat/tuple.py | 57 +++++++++---------- .../schemes/projective/projective_space.py | 4 +- 2 files changed, 30 insertions(+), 31 deletions(-) diff --git a/src/sage/combinat/tuple.py b/src/sage/combinat/tuple.py index 9002e5d7fb3..61449b98e00 100644 --- a/src/sage/combinat/tuple.py +++ b/src/sage/combinat/tuple.py @@ -35,23 +35,23 @@ class Tuples(Parent, UniqueRepresentation): sage: S = [1,2] sage: Tuples(S,3).list() - [[1, 1, 1], [2, 1, 1], [1, 2, 1], [2, 2, 1], [1, 1, 2], - [2, 1, 2], [1, 2, 2], [2, 2, 2]] + [(1, 1, 1), (2, 1, 1), (1, 2, 1), (2, 2, 1), (1, 1, 2), + (2, 1, 2), (1, 2, 2), (2, 2, 2)] sage: mset = ["s","t","e","i","n"] sage: Tuples(mset,2).list() - [['s', 's'], ['t', 's'], ['e', 's'], ['i', 's'], ['n', 's'], - ['s', 't'], ['t', 't'], ['e', 't'], ['i', 't'], ['n', 't'], - ['s', 'e'], ['t', 'e'], ['e', 'e'], ['i', 'e'], ['n', 'e'], - ['s', 'i'], ['t', 'i'], ['e', 'i'], ['i', 'i'], ['n', 'i'], - ['s', 'n'], ['t', 'n'], ['e', 'n'], ['i', 'n'], ['n', 'n']] + [('s', 's'), ('t', 's'), ('e', 's'), ('i', 's'), ('n', 's'), + ('s', 't'), ('t', 't'), ('e', 't'), ('i', 't'), ('n', 't'), + ('s', 'e'), ('t', 'e'), ('e', 'e'), ('i', 'e'), ('n', 'e'), + ('s', 'i'), ('t', 'i'), ('e', 'i'), ('i', 'i'), ('n', 'i'), + ('s', 'n'), ('t', 'n'), ('e', 'n'), ('i', 'n'), ('n', 'n')] :: sage: K. = GF(4, 'a') sage: mset = [x for x in K if x != 0] sage: Tuples(mset,2).list() - [[a, a], [a + 1, a], [1, a], [a, a + 1], [a + 1, a + 1], [1, a + 1], - [a, 1], [a + 1, 1], [1, 1]] + [(a, a), (a + 1, a), (1, a), (a, a + 1), (a + 1, a + 1), (1, a + 1), + (a, 1), (a + 1, 1), (1, 1)] """ @staticmethod def __classcall_private__(cls, S, k): @@ -94,22 +94,21 @@ def __iter__(self): sage: S = [1,2] sage: Tuples(S,3).list() - [[1, 1, 1], [2, 1, 1], [1, 2, 1], [2, 2, 1], [1, 1, 2], - [2, 1, 2], [1, 2, 2], [2, 2, 2]] + [(1, 1, 1), (2, 1, 1), (1, 2, 1), (2, 2, 1), (1, 1, 2), + (2, 1, 2), (1, 2, 2), (2, 2, 2)] sage: mset = ["s","t","e","i","n"] sage: Tuples(mset,2).list() - [['s', 's'], ['t', 's'], ['e', 's'], ['i', 's'], ['n', 's'], - ['s', 't'], ['t', 't'], ['e', 't'], ['i', 't'], - ['n', 't'], ['s', 'e'], ['t', 'e'], ['e', 'e'], ['i', 'e'], - ['n', 'e'], ['s', 'i'], ['t', 'i'], ['e', 'i'], - ['i', 'i'], ['n', 'i'], ['s', 'n'], ['t', 'n'], ['e', 'n'], - ['i', 'n'], ['n', 'n']] - sage: Tuples([1,1,2],3).list() - [[1, 1, 1], [2, 1, 1], [1, 2, 1], [2, 2, 1], [1, 1, 2], - [2, 1, 2], [1, 2, 2], [2, 2, 2]] + [('s', 's'), ('t', 's'), ('e', 's'), ('i', 's'), ('n', 's'), + ('s', 't'), ('t', 't'), ('e', 't'), ('i', 't'), ('n', 't'), + ('s', 'e'), ('t', 'e'), ('e', 'e'), ('i', 'e'), ('n', 'e'), + ('s', 'i'), ('t', 'i'), ('e', 'i'), ('i', 'i'), ('n', 'i'), + ('s', 'n'), ('t', 'n'), ('e', 'n'), ('i', 'n'), ('n', 'n')] + sage: Tuples((1,1,2),3).list() + [(1, 1, 1), (2, 1, 1), (1, 2, 1), (2, 2, 1), (1, 1, 2), + (2, 1, 2), (1, 2, 2), (2, 2, 2)] """ for p in product(self._index_list, repeat=self.k): - yield [self.S[i] for i in reversed(p)] + yield tuple(self.S[i] for i in reversed(p)) def cardinality(self): """ @@ -140,10 +139,10 @@ class UnorderedTuples(Parent, UniqueRepresentation): sage: S = [1,2] sage: UnorderedTuples(S,3).list() - [[1, 1, 1], [1, 1, 2], [1, 2, 2], [2, 2, 2]] + [(1, 1, 1), (1, 1, 2), (1, 2, 2), (2, 2, 2)] sage: UnorderedTuples(["a","b","c"],2).list() - [['a', 'a'], ['a', 'b'], ['a', 'c'], ['b', 'b'], ['b', 'c'], - ['c', 'c']] + [('a', 'a'), ('a', 'b'), ('a', 'c'), ('b', 'b'), ('b', 'c'), + ('c', 'c')] """ @staticmethod def __classcall_private__(cls, S, k): @@ -186,15 +185,15 @@ def __iter__(self): sage: S = [1,2] sage: UnorderedTuples(S,3).list() - [[1, 1, 1], [1, 1, 2], [1, 2, 2], [2, 2, 2]] + [(1, 1, 1), (1, 1, 2), (1, 2, 2), (2, 2, 2)] sage: UnorderedTuples(["a","b","c"],2).list() - [['a', 'a'], ['a', 'b'], ['a', 'c'], ['b', 'b'], ['b', 'c'], - ['c', 'c']] + [('a', 'a'), ('a', 'b'), ('a', 'c'), ('b', 'b'), ('b', 'c'), + ('c', 'c')] sage: UnorderedTuples([1,1,2],3).list() - [[1, 1, 1], [1, 1, 2], [1, 2, 2], [2, 2, 2]] + [(1, 1, 1), (1, 1, 2), (1, 2, 2), (2, 2, 2)] """ for ans in combinations_with_replacement(self._index_list, self.k): - yield [self.S[i] for i in ans] + yield tuple(self.S[i] for i in ans) def cardinality(self): """ diff --git a/src/sage/schemes/projective/projective_space.py b/src/sage/schemes/projective/projective_space.py index 221dca45981..67552b90ab8 100644 --- a/src/sage/schemes/projective/projective_space.py +++ b/src/sage/schemes/projective/projective_space.py @@ -2068,7 +2068,7 @@ def subscheme_from_Chow_form(self, Ch, dim): L1 = [] for t in UnorderedTuples(list(range(n + 1)), dim + 1): if all(t[i] < t[i + 1] for i in range(dim)): - L1.append(t) + L1.append(list(t)) # create the dual brackets L2 = [] signs = [] @@ -2374,7 +2374,7 @@ def rational_points(self, bound=0): for ai in R: P[i] = ai for tup in S[i - 1]: - if gcd([ai] + tup) == 1: + if gcd([ai] + list(tup)) == 1: for j in range(i): P[j] = tup[j] pts.append(self(P)) From 14e75408bd152d24979ff83e2772ab157f0b52a3 Mon Sep 17 00:00:00 2001 From: David Einstein Date: Sun, 25 Jun 2023 11:07:08 -0400 Subject: [PATCH 5/6] Switch tuple to list in projective_space.py Part of the ongoing project to free Tuple of GAP. --- src/sage/schemes/projective/projective_space.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sage/schemes/projective/projective_space.py b/src/sage/schemes/projective/projective_space.py index 67552b90ab8..744cb779fc4 100644 --- a/src/sage/schemes/projective/projective_space.py +++ b/src/sage/schemes/projective/projective_space.py @@ -2374,7 +2374,7 @@ def rational_points(self, bound=0): for ai in R: P[i] = ai for tup in S[i - 1]: - if gcd([ai] + list(tup)) == 1: + if gcd((ai,) + tup) == 1: for j in range(i): P[j] = tup[j] pts.append(self(P)) From 660653c7f07067c544aa3f65310a120903dbd532 Mon Sep 17 00:00:00 2001 From: David Einstein Date: Mon, 26 Jun 2023 16:11:09 -0400 Subject: [PATCH 6/6] Change dict to set The use of the python dict guarantee of insertion order of keys was confusing. There was no compelling reason (other than an aversion to change) for keeping insertion order. --- src/sage/combinat/tuple.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sage/combinat/tuple.py b/src/sage/combinat/tuple.py index 61449b98e00..17d285606dd 100644 --- a/src/sage/combinat/tuple.py +++ b/src/sage/combinat/tuple.py @@ -75,7 +75,7 @@ def __init__(self, S, k): """ self.S = S self.k = k - self._index_list = list(dict.fromkeys(S.index(s) for s in S)) + self._index_list = list(set(S.index(s) for s in S)) category = FiniteEnumeratedSets() Parent.__init__(self, category=category) @@ -166,7 +166,7 @@ def __init__(self, S, k): """ self.S = S self.k = k - self._index_list = list(dict.fromkeys(S.index(s) for s in S)) + self._index_list = list(set(S.index(s) for s in S)) category = FiniteEnumeratedSets() Parent.__init__(self, category=category)