#1155 closed defect (fixed)
[with patch, with positive review] PermutationGroup coercion bug
Reported by: | wdj | Owned by: | mhansen |
---|---|---|---|
Priority: | minor | Milestone: | sage-2.10.1 |
Component: | combinatorics | Keywords: | GAP, permutation group |
Cc: | sage-combinat | Merged in: | |
Authors: | Reviewers: | ||
Report Upstream: | 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 12 years ago by
- Milestone changed from sage-2.9 to sage-2.8.13
- Summary changed from PermutationGroup coercion bug to [with bundle] PermutationGroup coercion bug
comment:2 Changed 12 years ago by
- Summary changed from [with bundle] PermutationGroup coercion bug to [with bundle, with negative review] PermutationGroup coercion bug
comment:3 Changed 12 years ago by
Also, there are no doctests in the patch.
comment:4 Changed 12 years ago by
- Owner changed from wdj to mhansen
- Status changed from new to assigned
- Summary changed from [with bundle, with negative review] PermutationGroup coercion bug to [with patch] PermutationGroup coercion bug
I've put a new patch up.
Changed 12 years ago by
comment:5 Changed 12 years ago by
- Summary changed from [with patch] PermutationGroup coercion bug to [with patch, needs review] PermutationGroup coercion bug
comment:6 Changed 12 years ago by
- Summary changed from [with patch, needs review] PermutationGroup coercion bug to [with patch, with positive review] PermutationGroup coercion bug
Code looks good; doctest passes.
comment:7 Changed 12 years ago by
- Resolution set to fixed
- Status changed from assigned to closed
Merged in Sage 2.10.1.rc1
comment:8 Changed 11 years ago by
- Cc sage-combinat 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?