Skip to content

Commit

Permalink
Trac #27155: Fix and simplify libGAP error handling
Browse files Browse the repository at this point in the history
1. the usage of `PyErr_Fetch` in `error_handler()` is a memory leak: the
returned objects should be deallocated (Cython does not take care of
this since the type is `PyObject*` as opposed to `object`).

2. in various places in the libGAP interface, there is code of the form
{{{
        try:
            ...
        except RuntimeError as msg:
            raise ValueError('libGAP: '+str(msg))
}}}
It's not clear to me why we need to catch and re-raise an exception
here. Instead, we are raising this exception in our `error_handler`, so
we can just use a custom exception class `GAPError` to make it clear
that the error comes from GAP.

3. while I'm at it, I'm also replacing `libGAP` -> `GAP` in various
places and removing the obsolete `src/sage/libs/gap/test` test program.

URL: https://trac.sagemath.org/27155
Reported by: jdemeyer
Ticket author(s): Jeroen Demeyer
Reviewer(s): Erik Bray
  • Loading branch information
Release Manager authored and vbraun committed Feb 5, 2019
2 parents 3e7302b + 494ab6d commit 700570d
Show file tree
Hide file tree
Showing 23 changed files with 138 additions and 275 deletions.
4 changes: 2 additions & 2 deletions src/doc/en/reference/libs/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ libSingular
sage/libs/singular/ring
sage/libs/singular/groebner_strategy

libGAP
------
GAP
---
.. toctree::
:maxdepth: 2

Expand Down
2 changes: 1 addition & 1 deletion src/sage/combinat/combinat.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ def bell_number(n, algorithm='flint', **options):
- ``'flint'`` -- Wrap FLINT's ``arith_bell_number``
- ``'gap'`` -- Wrap libGAP's ``Bell``
- ``'gap'`` -- Wrap GAP's ``Bell``
- ``'mpmath'`` -- Wrap mpmath's ``bell``
Expand Down
2 changes: 1 addition & 1 deletion src/sage/combinat/sloane_functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ def _eval(self, n):
sage: sloane.A000001._eval(5000)
Traceback (most recent call last):
...
ValueError: libGAP: Error, the library of groups of size 5000 is not available
GAPError: Error, the library of groups of size 5000 is not available
"""
if n <= 50:
return self._small[n-1]
Expand Down
4 changes: 2 additions & 2 deletions src/sage/graphs/generators/classical_geometries.py
Original file line number Diff line number Diff line change
Expand Up @@ -761,7 +761,7 @@ def UnitaryDualPolarGraph(m, q):
sage: graphs.UnitaryDualPolarGraph(6,6)
Traceback (most recent call last):
...
ValueError: libGAP: Error, <subfield> must be a prime or a finite field
GAPError: Error, <subfield> must be a prime or a finite field
"""
from sage.libs.gap.libgap import libgap
G = _polar_graph(m, q**2, libgap.GeneralUnitaryGroup(m, q),
Expand Down Expand Up @@ -802,7 +802,7 @@ def SymplecticDualPolarGraph(m, q):
sage: graphs.SymplecticDualPolarGraph(6,6)
Traceback (most recent call last):
...
ValueError: libGAP: Error, <subfield> must be a prime or a finite field
GAPError: Error, <subfield> must be a prime or a finite field
REFERENCE:
Expand Down
4 changes: 2 additions & 2 deletions src/sage/groups/class_function.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class function on the conjugacy classes, in that order.
### GAP Interface-based Class Function
###
### This is old code that should be deleted once we have transitioned
### everything to libGAP.
### everything to using the library interface to GAP.
###
#####################################################################

Expand Down Expand Up @@ -802,7 +802,7 @@ def adams_operation(self, k):

#####################################################################
###
### libGAP-based Class function
### Class function using the GAP library
###
#####################################################################

Expand Down
2 changes: 1 addition & 1 deletion src/sage/groups/finitely_presented.py
Original file line number Diff line number Diff line change
Expand Up @@ -1263,7 +1263,7 @@ def semidirect_product(self, H, hom, check=True, reduced=False):
sage: D.semidirect_product(C, bad_hom)
Traceback (most recent call last):
...
ValueError: libGAP: Error, <gens> and <imgs> must be lists of same length
GAPError: Error, <gens> and <imgs> must be lists of same length
"""
from sage.groups.free_group import FreeGroup, _lexi_gen

Expand Down
4 changes: 2 additions & 2 deletions src/sage/groups/libgap_mixin.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
"""
Mix-in Class for libGAP-based Groups
Mix-in Class for GAP-based Groups
This class adds access to GAP functionality to groups such that parent
and element have a ``gap()`` method that returns a libGAP object for
and element have a ``gap()`` method that returns a GAP object for
the parent/element.
If your group implementation uses libgap, then you should add
Expand Down
10 changes: 5 additions & 5 deletions src/sage/groups/libgap_morphism.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
r"""
Group homomorphisms for groups with a libGAP backend
Group homomorphisms for groups with a GAP backend
EXAMPLES::
Expand Down Expand Up @@ -38,7 +38,7 @@

class GroupMorphism_libgap(Morphism):
r"""
This wraps libGAP group homomorphisms.
This wraps GAP group homomorphisms.
Checking if the input defines a group homomorphism can be expensive
if the group is large.
Expand All @@ -57,7 +57,7 @@ class GroupMorphism_libgap(Morphism):
sage: A.hom([g^2 for g in A.gens()])
Group endomorphism of Abelian group with gap, generator orders (2, 4)
Homomorphisms can be defined between different kinds of libGAP groups::
Homomorphisms can be defined between different kinds of GAP groups::
sage: G = MatrixGroup([Matrix(ZZ, 2, [0,1,1,0])])
sage: f = A.hom([G.0, G(1)])
Expand All @@ -76,7 +76,7 @@ class GroupMorphism_libgap(Morphism):
From: Free Group on generators {a, b}
To: Finitely presented group < a, b | a, b^3 >
Homomorphisms can be defined between libGAP groups and permutation groups::
Homomorphisms can be defined between GAP groups and permutation groups::
sage: S = Sp(4,3)
sage: P = PSp(4,3)
Expand Down Expand Up @@ -543,7 +543,7 @@ def preimage(self, S):
phi = self.gap()
from sage.groups.perm_gps.permgroup import PermutationGroup_generic
if not isinstance(S, (ParentLibGAP, PermutationGroup_generic)):
raise TypeError("%s must be a libGAP or permutation group of %s"%(S, self))
raise TypeError("%s must be a GAP or permutation group of %s"%(S, self))
if not self.codomain().gap().IsSubgroup(S.gap()).sage():
raise ValueError("%s must be a subgroup of %s"%(S, self))
preimage = phi.PreImage(S.gap())
Expand Down
2 changes: 1 addition & 1 deletion src/sage/groups/libgap_wrapper.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ This module provides helper class for wrapping GAP groups via
:mod:`~sage.libs.gap.libgap`. See :mod:`~sage.groups.free_group` for an
example how they are used.
The parent class keeps track of the libGAP element object, to use it
The parent class keeps track of the GAP element object, to use it
in your Python parent you have to derive both from the suitable group
parent and :class:`ParentLibGAP` ::
Expand Down
4 changes: 2 additions & 2 deletions src/sage/interfaces/gap.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,8 +226,8 @@ def set_gap_memory_pool_size(size_in_bytes):
"""
Set the desired gap memory pool size.
Subsequently started GAP/libGAP instances will use this as
default. Currently running instances are unchanged.
Subsequently started GAP instances will use this as default.
Already running instances are unchanged.
GAP will only reserve ``size_in_bytes`` address space. Unless you
actually start a big GAP computation, the memory will not be
Expand Down
2 changes: 1 addition & 1 deletion src/sage/interfaces/gap_workspace.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def gap_workspace_file(system="gap", name="workspace", dir=None):
``"libgap"``
- ``name`` -- the kind of workspace, usually ``"workspace"`` but
libGAP also uses other files
the library interface also uses other files
- ``dir`` -- the directory where the workspaces should be stored.
By default, this is ``DOT_SAGE/gap``
Expand Down
Loading

0 comments on commit 700570d

Please sign in to comment.