Skip to content
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.

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

PermutationGroup coercion bug #1155

Closed
wdjoyner opened this issue Nov 12, 2007 · 6 comments
Closed

PermutationGroup coercion bug #1155

wdjoyner opened this issue Nov 12, 2007 · 6 comments

Comments

@wdjoyner
Copy link

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)

CC: @sagetrac-sage-combinat

Component: combinatorics

Keywords: GAP, permutation group

Issue created by migration from https://trac.sagemath.org/ticket/1155

@wdjoyner wdjoyner added this to the sage-2.9 milestone Nov 12, 2007
@wdjoyner wdjoyner self-assigned this Nov 12, 2007
@sagetrac-mabshoff sagetrac-mabshoff mannequin modified the milestones: sage-2.9, sage-2.8.13 Nov 20, 2007
@sagetrac-cwitty
Copy link
Mannequin

sagetrac-cwitty mannequin commented Nov 27, 2007

comment:2

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?

@sagetrac-cwitty sagetrac-cwitty mannequin changed the title PermutationGroup coercion bug [with negative review] PermutationGroup coercion bug Nov 27, 2007
@sagetrac-cwitty
Copy link
Mannequin

sagetrac-cwitty mannequin commented Nov 27, 2007

comment:3

Also, there are no doctests in the patch.

@mwhansen
Copy link
Contributor

mwhansen commented Dec 6, 2007

comment:4

I've put a new patch up.

@mwhansen mwhansen assigned mwhansen and unassigned wdjoyner Dec 6, 2007
@mwhansen mwhansen changed the title [with negative review] PermutationGroup coercion bug PermutationGroup coercion bug Dec 6, 2007
@mwhansen
Copy link
Contributor

mwhansen commented Dec 6, 2007

Attachment: 1155.patch.gz

@rlmill rlmill mannequin added the s: needs review label Dec 22, 2007
@sagetrac-cwitty
Copy link
Mannequin

sagetrac-cwitty mannequin commented Jan 27, 2008

comment:6

Code looks good; doctest passes.

@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Jan 27, 2008

comment:7

Merged in Sage 2.10.1.rc1

@sagetrac-mabshoff sagetrac-mabshoff mannequin closed this as completed Jan 27, 2008
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants