#12407: Add the set of PrimitiveGroups
------------------------------------+---------------------------------------
Reporter: trehn | Owner: joyner
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-5.0
Component: group theory | Resolution:
Keywords: primitive groups | Work issues:
Report Upstream: N/A | Reviewers: vdelecroix
Authors: Thomas Rehn | Merged in:
Dependencies: | Stopgaps:
------------------------------------+---------------------------------------
Changes (by vdelecroix):
* status: needs_review => needs_work
Comment:
Hello,
0) There are trailing whitespaces in your file (spaces at the end of the
lines) that should be removed. All EXAMPLES statements should end with
'::' and a whiteline. Otherwise the examples are not well compiled in the
documentation. You put only one ':' at two places (line 1288, 1296). At
many places in the file permgroup_named this is the case... I asked a
question about this on sage-devel (http://groups.google.com/group/sage-
devel/browse_thread/thread/110e8595c86a62b)
1) At the very begining of the file permgroup_named.py there is a list of
all available groups. You should add yours.
2) In my previous remark 4), I claim that you should remove the very long
"number x of degree y" as its mathematical content is irrelevant. As you
can see the following list is ugly
{{{
sage: PrimitiveGroups(5).list()
[C(5), primitive group number 1 of degree 5, D(2*5), primitive group
number 2 of degree 5, AGL(1, 5), primitive group number 3 of degree 5,
A(5), primitive group number 4 of degree 5, S(5), primitive group number 5
of degree 5]
}}}
whereas calling gap directly we get the nicer
{{{
sage: [gap.PrimitiveGroup(5,i) for i in xrange(1,6)]
[C(5), D(2*5), AGL(1, 5), A(5), S(5)]
}}}
3) My previous remark 2) about naming convention is more dedicated for the
documentation of PrimitiveGroups, PrimitiveGroupsAll and
PrimitiveGroupsOfDegree than for PrimitiveGroup. As if a Sage user needs
it, it would be *before* the group is created!
4) In the examples of PrimitiveGroupsOfDegree, the actual example should
be simplified in the following way
{{{
sage: S = PrimitiveGroups(5); S # requires optional database_gap
Primitive Groups of degree 5
sage: S.list() # requires optional database_gap
[Primitive group number 1 of degree 5, Primitive group number 2 of degree
5,
Primitive group number 3 of degree 5, Primitive group number 4 of degree
5,
Primitive group number 5 of degree 5]
sage: S.an_element() # requires optional database_gap
Primitive group number 1 of degree 5
}}}
5) In the method cardinality of PrimitiveGroups you used once ZZ and once
Integer. I think that Integer is more dedicated. Could you remove ZZ from
the import statement at the begining of the file and replace ZZ by Integer
wherever it occurs (3 occurences counting yours).
6) The line 1508 (_an_element_ =
EnumeratedSets.ParentMethods._an_element_) makes no sense. The category
framework makes automatic import of ParentMethods. You may simply erase
it.
7) Your example line 1259 is a bit irrelevant: the string that goes with
the assertion error says "AssertionError: n should be in {1,..,1}".
8) In relation with my previous remark 4). You write a method
group_primitive_id, but in the EXAMPLES I get 2 instead of 4. Moreover,
you should complete it as
{{{
EXAMPLES::
sage: G = PermutationGroup([[(1,2,3,4,5)], [(1,5),(2,4)]])
sage: G.group_primitive_id() # optional - database_gap
2
sage: G.degree()
5
From the information of the degree and the identification number,
you can recover the isomorphism class of your group in the GAP
database::
sage: H = PermutationGroup(5,2) # optional - database_gap
sage: G == H # optional -
database_gap
False
sage: G.is_isomorphic(H) # optional - database_gap
True
}}}
9) Could you add a TODO in the documentation of PrimitiveGroups saying
that there exists a command PrimitiveGroupsIterator in GAP which accept
many arguments (nb. of moved points, size, transitivity, simple group,
solvable, ...).
oo) Typo:
- the INPUT documentation of the init method of PrimitiveGroup does not
follow the good syntax (PrimitiveGroups.__init__ is OK). You can have a
look at section 2.1.5 of the Developer Guide.
- how did you choose to break or not break lines in the documentation ?
It seems to be pretty random.
- there are some non needed empty lines: at the end of PrimitiveGroups
documentation and at the end of PrimitiveGroupsOfDegree documentation.
- line 1310, you wrote d up to isomorphism twice.
- Could you smartly write in all your classes that you wrap the GAP
database (it is important for the user to see that Sage uses another
software).
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/12407#comment:5>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
