#12339: Free Groups
--------------------------------+-------------------------------------------
Reporter: mmarco | Owner: joyner
Type: enhancement | Status: needs_review
Priority: minor | Milestone:
Component: group theory | Resolution:
Keywords: free groups | Work issues:
Report Upstream: N/A | Reviewers:
Authors: Miguel Marco | Merged in:
Dependencies: | Stopgaps:
--------------------------------+-------------------------------------------
Comment (by vbraun):
Looks great! Some libgap notes:
* libgap makes sure to lazy import itself, since gap startup takes some
time. Hence `sage/libs/all.py` defines
{{{
from sage.misc.lazy_import import lazy_import
lazy_import('sage.libs.gap.libgap', 'libgap')
}}}
You should either follow the same pattern and lazily import your own
code from `sage/groups/all.py`. This will help Sage in the long run to
avoid long startup times. Also, its better to import libgap as `from
sage.libs.all import libgap`, then you get the lazy importer.
There are a couple of code style and documentation issues:
* Always put an empty line before function/method declarations (that is,
before "def ...")
* Space after comma: `def __init__(self, ll=[], parent=None)` instead of
`def __init__(self,ll=[],parent=None)`
* Use
{{{
EXAMPLES::
sage: 1+1
}}}
instead of the multi-line
{{{
EXAMPLES:
::
sage: 1+1
}}}
* Its encouraged to generate intermediate variables instead of very long
lines:
{{{
self._gap_repr_=parent.gap().One().FamilyObj().ElementOfFpGroup(libgap.eval(str(l)).AbstractWordTietzeWord(parent.gap().FreeGroupOfFpGroup().GeneratorsOfGroup()))
}}}
would be more legible as
{{{
gap = parent.gap()
gens = gap.FreeGroupOfFpGroup().GeneratorsOfGroup()
element = gap.One().FamilyObj().ElementOfFpGroup(libgap.eval(str(l))
self._gap_rep = element.AbstractTietzeWord(gens)
}}}
* Module-level documentation for the new files, and add them to the
reference manual index (that is, to `doc/en/reference/groups.rst`) so the
documentation is actually built.
* Input parameters need to be documented consistently. That is,
consistently use `INPUT:` and `OUTPUT:` sections in the docstrings. You
know what the methods are for, but we don't ;-)
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/12339#comment:39>
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.
