#1155 closed defect (fixed)
[with patch, with positive review] PermutationGroup coercion bug
Reported by: | David Joyner | Owned by: | Mike Hansen |
---|---|---|---|
Priority: | minor | Milestone: | sage-2.10.1 |
Component: | combinatorics | Keywords: | GAP, permutation group |
Cc: | Sage Combinat CC user | Merged in: | |
Authors: | Reviewers: | ||
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
The patch http://sage.math.washington.edu/home/wdj/patches/permgp-2007-11-12.hg fixes a bug reported by Carlo H. Part of his email is pasted below:
+++++++++++++++++++++++++++++++++++++++++++++++
Hi,
I'm doing some work with groups. Using gap.Image() I can get a permutation like this:
sage: x (1,2)(3,7)(4,6)(5,8)
But to make a permutation group out of this element I have to enclose the x in two sets of brackets:
sage: PermutationGroup([[x]]) Permutation Group with generators [(1,2)(3,7)(4,6)(5,8)]
On the other hand, the following command fails (see below for code and output):
sage: PermutationGroup([x])
In my mind the second version is clearer - x is a permutation so [x] is a list of permutations and I should be able to use that to get a group.
Should SAGE do a coercion here, or am I doing something in a strange way?
Code and output:
---------------------------------------------------------------------- | SAGE Version 2.8.11, Release Date: 2007-11-02 | | Type notebook() for the GUI, and license() for information. | ---------------------------------------------------------------------- sage: p = 2 sage: assert is_prime(p) sage: sage: F = gap.new("FreeGroup(3)") sage: sage: a = F.gen(1) sage: b = F.gen(2) sage: c = F.gen(3) sage: sage: rels = [] sage: rels.append( a**Integer(p) ) sage: rels.append( b**Integer(p) ) sage: rels.append( c**Integer(p) ) sage: rels.append( a*b*((b*a*c)**Integer(-1)) ) sage: rels.append( c*a*((a*c)**Integer(-1)) ) sage: rels.append( c*b*((b*c)**Integer(-1)) ) sage: sage: N = gap.NormalClosure(F, gap.Subgroup(F, rels)) sage: niso = gap.NaturalHomomorphismByNormalSubgroupNC(F, N) sage: sage: x = gap.Image(niso, a) sage: x (1,2)(3,7)(4,6)(5,8) sage: PermutationGroup([[x]]) Permutation Group with generators [(1,2)(3,7)(4,6)(5,8)] sage: sage: PermutationGroup([x]) --------------------------------------------------------------------------- <type 'exceptions.TypeError'> Traceback (most recent call last)
Attachments (1)
Change History (9)
comment:1 Changed 15 years ago by
Milestone: | sage-2.9 → sage-2.8.13 |
---|---|
Summary: | PermutationGroup coercion bug → [with bundle] PermutationGroup coercion bug |
comment:2 Changed 15 years ago by
Summary: | [with bundle] PermutationGroup coercion bug → [with bundle, with negative review] PermutationGroup coercion bug |
---|
comment:4 Changed 15 years ago by
Owner: | changed from David Joyner to Mike Hansen |
---|---|
Status: | new → assigned |
Summary: | [with bundle, with negative review] PermutationGroup coercion bug → [with patch] PermutationGroup coercion bug |
I've put a new patch up.
Changed 15 years ago by
Attachment: | 1155.patch added |
---|
comment:5 Changed 15 years ago by
Summary: | [with patch] PermutationGroup coercion bug → [with patch, needs review] PermutationGroup coercion bug |
---|
comment:6 Changed 15 years ago by
Summary: | [with patch, needs review] PermutationGroup coercion bug → [with patch, with positive review] PermutationGroup coercion bug |
---|
Code looks good; doctest passes.
comment:7 Changed 15 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Merged in Sage 2.10.1.rc1
comment:8 Changed 14 years ago by
Cc: | Sage Combinat CC user added |
---|
This patch doesn't feel right to me; it seems like it's fixing the problem at the wrong level. (For instance, it sometimes breaks if you try to create a permutation group from a list of generators where some of the generators are Python lists and some are Gap permutation group elements. Maybe that's too strange a case to worry about?)
I haven't tried it, but it looks like adding a special case to gap_format() for Gap permutation group elements would also fix the problem, perhaps in a better way?