#13251: Add General Dihedral Groups as Permutation Groups
--------------------------------+-------------------------------------------
       Reporter:  khalasz       |         Owner:  joyner        
           Type:  enhancement   |        Status:  needs_review  
       Priority:  minor         |     Milestone:  sage-5.3      
      Component:  group theory  |    Resolution:                
       Keywords:                |   Work issues:                
Report Upstream:  N/A           |     Reviewers:  Benjamin Jones
        Authors:  Kevin Halasz  |     Merged in:                
   Dependencies:                |      Stopgaps:                
--------------------------------+-------------------------------------------
Changes (by benjaminfjones):

  * reviewer:  => Benjamin Jones


Comment:

 Looks great. I found a few typos while reading the doc strings. I'll look
 at the _init_ code more carefully, later, but on first glance and playing
 around with it for a few minutes it seems solid.

 Typos to check in `sage/groups/perm_gps/permgroup_named.py`:

  * Line 767: The first sentence here is not a sentence.
  * Line 770: replace "factor" with "product"
  * Line 776: What do you mean by "degree" here? Do you mean "order"? If
 so, I would say that or else clarify degree.
  * `__init__()` needs a docstring. In these cases I tend to write
 something like

 {{{
 def __init__():
     r"""
     Init method of class <CLASSNAME>. See the docstring for
 :class:`<CLASSNAME>`.

     EXAMPLES::

         sage: ex1
     """
     ...
 }}}

 Nice work, I'm glad this is on its way to getting into Sage. More later..

-- 
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/13251#comment:2>
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.

Reply via email to