From 683513bc88e8e40bb36d63072c4eba55c499caea Mon Sep 17 00:00:00 2001 From: RuchitJagodara Date: Sun, 11 Feb 2024 01:10:28 +0530 Subject: [PATCH 01/12] Fixed the problem with direct making a permutation from a permutation group element --- src/sage/combinat/permutation.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/sage/combinat/permutation.py b/src/sage/combinat/permutation.py index 92e187287fd..0bdbacedf66 100644 --- a/src/sage/combinat/permutation.py +++ b/src/sage/combinat/permutation.py @@ -7092,7 +7092,17 @@ def _element_constructor_(self, x, check=True): [1, 4, 5, 2, 3, 6] sage: Permutations(6)(x) # known bug [1, 4, 5, 2, 3, 6] + + check if :trac:`37284` is fixed:: + + sage: S5 = SymmetricGroup(5) + sage: P5 = Permutations(5) + sage: p = S5.list()[3] + sage: P5(p) + [4, 5, 1, 2, 3] """ + if not isinstance(x, Permutation): + x = Permutation(x) if len(x) < self.n: x = list(x) + list(range(len(x) + 1, self.n + 1)) return self.element_class(self, x, check=check) From 90971f0b1256015d5692fd6bddfdca4edfe30f6f Mon Sep 17 00:00:00 2001 From: RuchitJagodara Date: Sun, 11 Feb 2024 01:48:16 +0530 Subject: [PATCH 02/12] Made the changes efficient --- src/sage/combinat/permutation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sage/combinat/permutation.py b/src/sage/combinat/permutation.py index 0bdbacedf66..fd82d0ab75d 100644 --- a/src/sage/combinat/permutation.py +++ b/src/sage/combinat/permutation.py @@ -7102,7 +7102,7 @@ def _element_constructor_(self, x, check=True): [4, 5, 1, 2, 3] """ if not isinstance(x, Permutation): - x = Permutation(x) + x = x.domain() if len(x) < self.n: x = list(x) + list(range(len(x) + 1, self.n + 1)) return self.element_class(self, x, check=check) From 7e1f3fc0353ce16299dd1c012547507dba3823b5 Mon Sep 17 00:00:00 2001 From: RuchitJagodara Date: Sun, 11 Feb 2024 14:57:45 +0530 Subject: [PATCH 03/12] Added try except block to cover all cases --- src/sage/combinat/permutation.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/sage/combinat/permutation.py b/src/sage/combinat/permutation.py index fd82d0ab75d..ec1fe0dd406 100644 --- a/src/sage/combinat/permutation.py +++ b/src/sage/combinat/permutation.py @@ -7101,8 +7101,11 @@ def _element_constructor_(self, x, check=True): sage: P5(p) [4, 5, 1, 2, 3] """ - if not isinstance(x, Permutation): - x = x.domain() + try: + if not isinstance(x, Permutation): + x = x.domain() + except AttributeError: + pass if len(x) < self.n: x = list(x) + list(range(len(x) + 1, self.n + 1)) return self.element_class(self, x, check=check) From b7b05c65bcc035f70e30ef17baaf5bf9c92426b5 Mon Sep 17 00:00:00 2001 From: Ruchit Jagodara Date: Mon, 12 Feb 2024 19:33:53 +0530 Subject: [PATCH 04/12] Check for specific type while x is not in correct form Co-authored-by: Travis Scrimshaw --- src/sage/combinat/permutation.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/sage/combinat/permutation.py b/src/sage/combinat/permutation.py index ec1fe0dd406..fb2ec9afc6b 100644 --- a/src/sage/combinat/permutation.py +++ b/src/sage/combinat/permutation.py @@ -7101,11 +7101,8 @@ def _element_constructor_(self, x, check=True): sage: P5(p) [4, 5, 1, 2, 3] """ - try: - if not isinstance(x, Permutation): - x = x.domain() - except AttributeError: - pass + if isinstance(x, PermutationGroupElement): + x = x.domain() if len(x) < self.n: x = list(x) + list(range(len(x) + 1, self.n + 1)) return self.element_class(self, x, check=check) From f444d326540464fa9908fcd73fd3ab16511c5410 Mon Sep 17 00:00:00 2001 From: Ruchit Jagodara Date: Mon, 12 Feb 2024 19:34:36 +0530 Subject: [PATCH 05/12] Corrected the test documentation Co-authored-by: grhkm21 <83517584+grhkm21@users.noreply.github.com> --- src/sage/combinat/permutation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sage/combinat/permutation.py b/src/sage/combinat/permutation.py index fb2ec9afc6b..1e213f2d5ad 100644 --- a/src/sage/combinat/permutation.py +++ b/src/sage/combinat/permutation.py @@ -7093,7 +7093,7 @@ def _element_constructor_(self, x, check=True): sage: Permutations(6)(x) # known bug [1, 4, 5, 2, 3, 6] - check if :trac:`37284` is fixed:: + Ensure that :issue:`37284` is fixed:: sage: S5 = SymmetricGroup(5) sage: P5 = Permutations(5) From 9ced7837d2c884d1fe0d5a1407c79c3ea007b6e6 Mon Sep 17 00:00:00 2001 From: Ruchit Jagodara Date: Thu, 15 Feb 2024 21:47:45 +0530 Subject: [PATCH 06/12] Use _from_permutation_group_element function instead of updating x Co-authored-by: Travis Scrimshaw --- src/sage/combinat/permutation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sage/combinat/permutation.py b/src/sage/combinat/permutation.py index 1e213f2d5ad..665faba8985 100644 --- a/src/sage/combinat/permutation.py +++ b/src/sage/combinat/permutation.py @@ -7102,7 +7102,7 @@ def _element_constructor_(self, x, check=True): [4, 5, 1, 2, 3] """ if isinstance(x, PermutationGroupElement): - x = x.domain() + return self. _from_permutation_group_element(x) if len(x) < self.n: x = list(x) + list(range(len(x) + 1, self.n + 1)) return self.element_class(self, x, check=check) From 7328410da91d8001e338c959ac812bf8e81e6451 Mon Sep 17 00:00:00 2001 From: RuchitJagodara Date: Thu, 15 Feb 2024 21:51:52 +0530 Subject: [PATCH 07/12] Updated the test case --- src/sage/combinat/permutation.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/sage/combinat/permutation.py b/src/sage/combinat/permutation.py index 665faba8985..748f3197177 100644 --- a/src/sage/combinat/permutation.py +++ b/src/sage/combinat/permutation.py @@ -7095,11 +7095,11 @@ def _element_constructor_(self, x, check=True): Ensure that :issue:`37284` is fixed:: - sage: S5 = SymmetricGroup(5) + sage: PG = PermutationGroup(SymmetricGroup(5).gens()) sage: P5 = Permutations(5) - sage: p = S5.list()[3] - sage: P5(p) - [4, 5, 1, 2, 3] + sage: p = PG.list()[0] + sage: s = P5(p); s + [1, 2, 3, 4, 5] """ if isinstance(x, PermutationGroupElement): return self. _from_permutation_group_element(x) From f8d9d70d8623ad91f5ecab561f48c537f1ec37d1 Mon Sep 17 00:00:00 2001 From: RuchitJagodara Date: Sun, 18 Feb 2024 15:19:45 +0530 Subject: [PATCH 08/12] Improve the quality of test case --- src/sage/combinat/permutation.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/sage/combinat/permutation.py b/src/sage/combinat/permutation.py index 748f3197177..12de6e95165 100644 --- a/src/sage/combinat/permutation.py +++ b/src/sage/combinat/permutation.py @@ -7097,9 +7097,10 @@ def _element_constructor_(self, x, check=True): sage: PG = PermutationGroup(SymmetricGroup(5).gens()) sage: P5 = Permutations(5) - sage: p = PG.list()[0] - sage: s = P5(p); s - [1, 2, 3, 4, 5] + sage: p = PG.an_element() + sage: x = PG([4,3,5,1,2]) + sage: s = P5(x); s + [4, 3, 5, 1, 2] """ if isinstance(x, PermutationGroupElement): return self. _from_permutation_group_element(x) From 0d2a9445f0479b6ae00751edd9d3095d583c5f4c Mon Sep 17 00:00:00 2001 From: RuchitJagodara Date: Sun, 18 Feb 2024 15:20:46 +0530 Subject: [PATCH 09/12] Added a line in test case to ensure that conversion was successfull --- src/sage/combinat/permutation.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/sage/combinat/permutation.py b/src/sage/combinat/permutation.py index 12de6e95165..fb67129e3d8 100644 --- a/src/sage/combinat/permutation.py +++ b/src/sage/combinat/permutation.py @@ -7101,6 +7101,8 @@ def _element_constructor_(self, x, check=True): sage: x = PG([4,3,5,1,2]) sage: s = P5(x); s [4, 3, 5, 1, 2] + sage: s.parent() + Standard permutations of 5 """ if isinstance(x, PermutationGroupElement): return self. _from_permutation_group_element(x) From c4a60bde89393e24404c3c631d5e226529d440e5 Mon Sep 17 00:00:00 2001 From: RuchitJagodara Date: Sun, 25 Feb 2024 23:09:42 +0530 Subject: [PATCH 10/12] Make test more robust for the function _element_constructor_ --- src/sage/combinat/permutation.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/sage/combinat/permutation.py b/src/sage/combinat/permutation.py index fb67129e3d8..e26bde5e0cc 100644 --- a/src/sage/combinat/permutation.py +++ b/src/sage/combinat/permutation.py @@ -7095,14 +7095,11 @@ def _element_constructor_(self, x, check=True): Ensure that :issue:`37284` is fixed:: - sage: PG = PermutationGroup(SymmetricGroup(5).gens()) - sage: P5 = Permutations(5) + sage: PG = PermutationGroup([[(1,2,3),(5,6)],[(7,8)]]) + sage: P5 = Permutations(8) sage: p = PG.an_element() - sage: x = PG([4,3,5,1,2]) - sage: s = P5(x); s - [4, 3, 5, 1, 2] - sage: s.parent() - Standard permutations of 5 + sage: P5(p).parent() + Standard permutations of 8 """ if isinstance(x, PermutationGroupElement): return self. _from_permutation_group_element(x) From 6bc3152598a045a10c969dd5e768c2b12c0f064f Mon Sep 17 00:00:00 2001 From: Ruchit Jagodara Date: Mon, 26 Feb 2024 18:08:16 +0530 Subject: [PATCH 11/12] Correct variable names in test Co-authored-by: Travis Scrimshaw --- src/sage/combinat/permutation.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sage/combinat/permutation.py b/src/sage/combinat/permutation.py index e26bde5e0cc..0bd2f33549c 100644 --- a/src/sage/combinat/permutation.py +++ b/src/sage/combinat/permutation.py @@ -7096,9 +7096,9 @@ def _element_constructor_(self, x, check=True): Ensure that :issue:`37284` is fixed:: sage: PG = PermutationGroup([[(1,2,3),(5,6)],[(7,8)]]) - sage: P5 = Permutations(8) + sage: P8 = Permutations(8) sage: p = PG.an_element() - sage: P5(p).parent() + sage: P8(p).parent() Standard permutations of 8 """ if isinstance(x, PermutationGroupElement): From 54747b616a71a70cd8206636c06fc458c0661e16 Mon Sep 17 00:00:00 2001 From: RuchitJagodara Date: Mon, 26 Feb 2024 18:15:14 +0530 Subject: [PATCH 12/12] Make tests more robust --- src/sage/combinat/permutation.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/sage/combinat/permutation.py b/src/sage/combinat/permutation.py index 0bd2f33549c..df41747f5b2 100644 --- a/src/sage/combinat/permutation.py +++ b/src/sage/combinat/permutation.py @@ -7098,7 +7098,9 @@ def _element_constructor_(self, x, check=True): sage: PG = PermutationGroup([[(1,2,3),(5,6)],[(7,8)]]) sage: P8 = Permutations(8) sage: p = PG.an_element() - sage: P8(p).parent() + sage: q = P8(p); q + [2, 3, 1, 4, 6, 5, 8, 7] + sage: q.parent() Standard permutations of 8 """ if isinstance(x, PermutationGroupElement):