#8380: Implement an interface to GAP3
------------------------------+---------------------------------------------
Reporter: saliola | Owner: Franco Saliola
Type: enhancement | Status: needs_review
Priority: major | Milestone: sage-4.4.2
Component: interfaces | Keywords: gap3, chevie, specht, gap,
sage-combinat
Author: Franco Saliola | Upstream: N/A
Reviewer: Burcin Erocal | Merged:
Work_issues: |
------------------------------+---------------------------------------------
Changes (by saliola):
* status: needs_work => needs_review
Comment:
I've uploaded my changes in a separate patch to ease the review. Apply the
patches in this order:
* attachment:gap3_interface_v4.3.3.patch
* attachment:gap3_interface_patch2.patch
Replying to [comment:11 burcin]:
> Here is my review for the patch:
> * There is no doctest for the change in `sage/interfaces/expect.py`
The problem here was that a variable name could be overwritten; before the
patch:
{{{
sage: x = gap(3)
sage: gap.clear(x.name())
sage: gap.clear(x.name())
sage: x = gap(3); x
3
sage: y = gap(4); y
4
sage: x # this should be 3!
4
}}}
This is now corrected, and I added the above as a doctest.
> * The method `load_package()` in `sage/interfaces/gap.py` doesn't have
a doctest. I understand that this is copied as is from the old version,
but if there is any package that is included by default in the GAP4
distribution (or one which we include in our package), we should add a
test.
Neither Sage nor Gap seem to distribute any packages (see the
{{{$SAGE_ROOT/local/lib/gap-4.4.12/pkg}}} directory). I did, however,
add a test that at least tests that it raises an appropriate exception.
> * In `sage/interfaces/gap3.py`
> * Is the bug in the pexpect interface mentioned around line 42
reported on trac? Can you mention the ticket number in that comment. Is
this specific to the GAP interface?
Yes, it is #8471. I added the ticket number to the comment, and
cross-referenced that ticket to this one.
It is not specific to the GAP interface. It is an issue with any
{{{Expect}}} instance. See #8471 for details.
> * does the GAP3 banner depend on the specific package installed?
It shouldn't since the software is so old. Note that when the banners are
printed in the documentation, it is only for illustration purposes. Those
commands are not tested because each spawns a console (which would require
user input to quit).
> * There are some doctests that depend on chevie,
(`RequirePackage('"chevie"')` and `load_package("chevie")`), these should
be optional.
I've marked them as {{{#optional - gap3chevie}}} instead of just
{{{#optional - gap3}}}.
> * The docstring for `GAP3Record.__getattr__` ends with " :: " then an
empty line. There are many places where there is an empty line at the end
of the docstring, or right after.
Corrected.
> The optional package for gap3 in comment:9 looks good in general. Maybe
the fact that it's binary only can be made more obvious, for example by
adding a `bin` to the package name.
The discussion surrounding spkgs should be moved to #8906, which proposes
a source gap3 spkg instead.
> BTW, it's not possible to install the version of GAP3 downloaded from
the main web site (http://www.gap-system.org/Gap3/Download3/download.html)
easily. I suggest moving the link to Frank Luebeck's distribution to the
first place, and putting this option last.
Done. I listed ticket #8906 as the first option (it should be changed when
that ticket is resolved).
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/8380#comment:15>
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.
