Skip to content
This repository has been archived by the owner on Jan 30, 2023. It is now read-only.

Commit

Permalink
Deprecate sage.libs.gap.element and convert Sage to use
Browse files Browse the repository at this point in the history
gappy.gapobj.GapObj directly as the primary wrapper for GAP objects.

The sage.libs.gap.element module is kept for now but deprecated, and
GapElement and all its subclasses are just aliases for the equivalent
GapObjs, so existing code using these classes might still work as long
as they're not using them at the C/Cython level.  So it's not perfect
backwards-compatibility but it's better than nothing.

By necessity this is something of a patchbomb, but most of the changes
are simply changing GapElement -> GapObj.  A few changes deserve
specific mention, however:

* Added sage.libs.gap.converters--this uses gappy's GapObj.convert_to
  API to add `.sage()` methods to most of the GapObj subclasses, so they
  behave the same as their GapElement progenitors w.r.t. conversion to
  equivalent Sage types.

* Integer, Rational, and IntegerMod all have explicit cases in their
  constructors for construction from equivalent GapObjs.

* As mentioned in the ticket #31404, Polynomial.__call__ has a special
  case for handling GapObjs, since nothing else can be coerced to them
  when evaluating a polynomial on a GapObj.

* MatrixArgs now has an explicit case for converting GapLists to Sage
  matrices.

* sage.groups.class_function had some doctests for ClassFunction_libgap
  which were copy/pasted from ClassFunction_gap without modification,
  so it was not properly testing the libgap implementation of
  ClassFunction; this has been fixed

* Added an explicit _libgap_ method for efficient conversion of
  NumberFields to their GAP equivalents.  Also cleaned up the existing
  _gap_ implementation.

* Fixed a bit of code in
  sage.groups.matrix_gps.matrix_group.MatrixGroup_gap that was using
  libgap inefficiently.

* Ditto in sage.groups.perm_gps.partn_ref2.refinement_generic

* Removed the late_import stuff from sage.rings.universal_cyclotomic_field
  since it does not appear to be necessary any longer.

* In PermutationGroup_generic some doctests which compared libgap
  objects to gap (pexpect) objects had to be changed--previously they
  did not compare as equal, but as a surprising but welcome change
  (perhaps due to going outside the coercion system) GapObjs now compare
  equal to their equivalent pexect GapElements.

* Updated more tests to account for slight differences in iteration
  order on groups.
  • Loading branch information
embray committed Feb 26, 2021
1 parent b305f39 commit 86e4b1b
Show file tree
Hide file tree
Showing 38 changed files with 1,176 additions and 3,472 deletions.
8 changes: 5 additions & 3 deletions src/sage/groups/abelian_gps/abelian_group_gap.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,15 @@
from sage.groups.libgap_wrapper import ParentLibGAP, ElementLibGAP
from sage.groups.libgap_mixin import GroupMixinLibGAP
from sage.groups.group import AbelianGroup as AbelianGroupBase
from sage.libs.gap.element import GapElement
from sage.libs.gap.libgap import libgap
from sage.misc.cachefunc import cached_method
from sage.rings.integer_ring import ZZ
from sage.structure.unique_representation import UniqueRepresentation
from sage.categories.groups import Groups

from gappy.gapobj import GapObj


class AbelianGroupElement_gap(ElementLibGAP):
r"""
An element of an abelian group via libgap.
Expand All @@ -59,7 +61,7 @@ def __init__(self, parent, x, check=True):
INPUT:
- ``parent`` -- an instance of :class:`AbelianGroup_gap`
- ``x`` -- an instance of :class:`sage.libs.gap.element.GapElement`
- ``x`` -- an instance of :class:`gappy.gapobj.GapObj`
- ``check`` -- boolean (default: ``True``); check
if ``x`` is an element of the group
Expand Down Expand Up @@ -337,7 +339,7 @@ def _element_constructor_(self, x, check=True):
x = x.gap()
elif x == 1 or x == ():
x = self.gap().Identity()
elif not isinstance(x, GapElement):
elif not isinstance(x, GapObj):
from sage.groups.abelian_gps.abelian_group_element import AbelianGroupElement
from sage.groups.additive_abelian.additive_abelian_group import AdditiveAbelianGroupElement
from sage.modules.fg_pid.fgp_element import FGP_Element
Expand Down
17 changes: 8 additions & 9 deletions src/sage/groups/class_function.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@

from sage.structure.sage_object import SageObject
from sage.structure.richcmp import richcmp, richcmp_method
from sage.interfaces.gap import gap
from sage.interfaces.gap import gap, GapElement
from sage.rings.all import Integer
from sage.rings.all import CyclotomicField
from sage.libs.gap.element import GapElement
from sage.libs.gap.libgap import libgap
from sage.libs.gap.element import GapElement as LibGapElement

from gappy.gapobj import GapObj

# TODO:
#
Expand Down Expand Up @@ -68,7 +68,7 @@ class function on the conjugacy classes, in that order.
except AttributeError:
pass

if isinstance(values, LibGapElement):
if isinstance(values, GapObj):
return ClassFunction_libgap(group, values)

return ClassFunction_gap(group, values)
Expand Down Expand Up @@ -847,7 +847,7 @@ def __init__(self, G, values):
Character of Cyclic group of order 4 as a permutation group
"""
self._group = G
if isinstance(values, LibGapElement) and values.IsClassFunction():
if isinstance(values, GapObj) and values.IsClassFunction():
self._gap_classfunction = values
else:
self._gap_classfunction = libgap.ClassFunction(G._libgap_(),
Expand All @@ -868,16 +868,15 @@ def gap(self):
Character of Cyclic group of order 4 as a permutation group
sage: type(chi)
<class 'sage.groups.class_function.ClassFunction_gap'>
sage: gap(chi)
ClassFunction( CharacterTable( Group( [ (1,2,3,4) ] ) ), [ 1, -1, 1, -1 ] )
sage: libgap(chi)
ClassFunction( CharacterTable( Group([ (1,2,3,4) ]) ), [ 1, -1, 1, -1 ] )
sage: type(_)
<class 'sage.interfaces.gap.GapElement'>
<class 'gappy.gapobj.GapList'>
"""
return self._gap_classfunction

_libgap_ = _gap_ = gap


def _repr_(self):
r"""
Return a string representation.
Expand Down
7 changes: 3 additions & 4 deletions src/sage/groups/cubic_braid.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@


from sage.misc.cachefunc import cached_method
from sage.libs.gap.element import GapElement
from sage.groups.free_group import FreeGroup
from sage.groups.finitely_presented import FinitelyPresentedGroup, FinitelyPresentedGroupElement
from sage.groups.braid import BraidGroup
Expand All @@ -91,6 +90,8 @@
from sage.rings.finite_rings.finite_field_constructor import GF
from sage.structure.element import Element

from gappy.gapobj import GapObj

from enum import Enum


Expand Down Expand Up @@ -263,15 +264,13 @@ def __init__(self, parent, x, check=True):
"""
if type(x) in (tuple, list):
x = tuple(_reduce_tietze(tuple(x)))
elif isinstance(x, GapElement):
elif isinstance(x, GapObj):
tietze_list = x.UnderlyingElement().TietzeWordAbstractWord().sage()
tietze_red = _reduce_tietze(tietze_list)
if tietze_red != tietze_list:
x = tuple(tietze_red)
super(CubicBraidElement, self).__init__(parent, x, check=check)



def _richcmp_(self, other, op):
"""
overwrite comparison since the inherited one from FinitelyPresentedGroupElement
Expand Down
41 changes: 21 additions & 20 deletions src/sage/groups/finitely_presented.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@
sage: a.gap().Order()
2
sage: type(_) # note that the above output is not a Sage integer
<type 'sage.libs.gap.element.GapElement_Integer'>
<class 'gappy.gapobj.GapInteger'>
You can use call syntax to replace the generators with a set of
arbitrary ring elements. For example, take the free abelian group
Expand Down Expand Up @@ -133,22 +133,23 @@
from sage.groups.libgap_mixin import GroupMixinLibGAP
from sage.structure.unique_representation import UniqueRepresentation
from sage.libs.gap.libgap import libgap
from sage.libs.gap.element import GapElement
from sage.misc.cachefunc import cached_method
from sage.groups.free_group import FreeGroupElement
from sage.functions.generalized import sign
from sage.matrix.constructor import matrix
from sage.categories.morphism import SetMorphism

from gappy.gapobj import GapObj


class GroupMorphismWithGensImages(SetMorphism):
r"""
Class used for morphisms from finitely presented groups to
other groups. It just adds the images of the generators at the
end of the representation.
EXAMPLES::
sage: F = FreeGroup(3)
sage: G = F / [F([1, 2, 3, 1, 2, 3]), F([1, 1, 1])]
sage: H = AlternatingGroup(3)
Expand All @@ -166,9 +167,9 @@ class GroupMorphismWithGensImages(SetMorphism):
def _repr_defn(self):
r"""
Return the part of the representation that includes the images of the generators.
EXAMPLES::
sage: F = FreeGroup(3)
sage: G = F / [F([1,2,3,1,2,3]),F([1,1,1])]
sage: H = AlternatingGroup(3)
Expand Down Expand Up @@ -230,7 +231,7 @@ def __init__(self, parent, x, check=True):
sage: TestSuite(x).run()
sage: TestSuite(G.one()).run()
"""
if not isinstance(x, GapElement):
if not isinstance(x, GapObj):
F = parent.free_group()
free_element = F(x)
fp_family = parent.gap().Identity().FamilyObj()
Expand Down Expand Up @@ -286,7 +287,7 @@ def _repr_(self):
if self.Tietze() == ():
return '1'
else:
return self.gap()._repr_()
return self.gap().__repr__()

@cached_method
def Tietze(self):
Expand Down Expand Up @@ -376,9 +377,9 @@ def wrap_FpGroup(libgap_fpgroup):
``libgap_free_group`` to comparison by Python ``id``. If you want
to put the LibGAP free group into a container ``(set, dict)`` then you
should understand the implications of
:meth:`~sage.libs.gap.element.GapElement._set_compare_by_id`. To
be safe, it is recommended that you just work with the resulting
Sage :class:`FinitelyPresentedGroup`.
:meth:`~gappy.gapobj.GapObj._set_compare_by_id`. To be safe, it is
recommended that you just work with the resulting Sage
:class:`FinitelyPresentedGroup`.
INPUT:
Expand All @@ -397,7 +398,7 @@ def wrap_FpGroup(libgap_fpgroup):
sage: P = F / libgap([ a_cubed ]); P
<fp group of size infinity on the generators [ a, b ]>
sage: type(P)
<type 'sage.libs.gap.element.GapElement'>
<class 'gappy.gapobj.GapObj'>
Now wrap it::
Expand Down Expand Up @@ -775,7 +776,7 @@ class FinitelyPresentedGroup(GroupMixinLibGAP, UniqueRepresentation,
sage: H.gap()
<fp group on the generators [ x0, x1 ]>
sage: type(_)
<type 'sage.libs.gap.element.GapElement'>
<class 'gappy.gapobj.GapObj'>
"""
Element = FinitelyPresentedGroupElement

Expand Down Expand Up @@ -827,7 +828,7 @@ def _repr_(self):
"""
gens = ', '.join(self.variable_names())
rels = ', '.join([ str(r) for r in self.relations() ])
return 'Finitely presented group ' + '< '+ gens + ' | ' + rels + ' >'
return f'Finitely presented group < {gens} | {rels} >'

def _latex_(self):
"""
Expand Down Expand Up @@ -1426,17 +1427,17 @@ def simplified(self):
Finitely presented group < e0 | e0^2 >
"""
return self.simplification_isomorphism().codomain()

def epimorphisms(self, H):
r"""
Return the epimorphisms from `self` to `H`, up to automorphism of `H`.
INPUT:
- `H` -- Another group
EXAMPLES::
sage: F = FreeGroup(3)
sage: G = F / [F([1, 2, 3, 1, 2, 3]), F([1, 1, 1])]
sage: H = AlternatingGroup(3)
Expand Down Expand Up @@ -1467,7 +1468,7 @@ def epimorphisms(self, H):
x2 |--> (1,2,3)]
ALGORITHM:
Uses libgap's GQuotients function.
"""
from sage.misc.misc_c import prod
Expand Down
5 changes: 3 additions & 2 deletions src/sage/groups/fqf_orthogonal.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@
from sage.matrix.all import matrix
from sage.categories.action import Action

from gappy.gapobj import GapObj


class FqfIsometry(AbelianGroupAutomorphism):
r"""
Expand Down Expand Up @@ -251,8 +253,7 @@ def _element_constructor_(self, x, check=True):
sage: assert Oq(OL.0) == Oq(OL.0.matrix())
sage: assert Oq(Oq.0.matrix()) == Oq.0
"""
from sage.libs.gap.element import GapElement
if not isinstance(x, GapElement):
if not isinstance(x, GapObj):
try:
# if there is an action try that
gen = self.invariant_form().smith_form_gens()
Expand Down
17 changes: 9 additions & 8 deletions src/sage/groups/free_group.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,15 @@
from sage.groups.libgap_wrapper import ParentLibGAP, ElementLibGAP
from sage.structure.unique_representation import UniqueRepresentation
from sage.libs.gap.libgap import libgap
from sage.libs.gap.element import GapElement
from sage.rings.integer import Integer
from sage.rings.integer_ring import IntegerRing
from sage.misc.cachefunc import cached_method
from sage.misc.misc_c import prod
from sage.structure.sequence import Sequence
from sage.structure.element import coercion_model, parent

from gappy.gapobj import GapObj


def is_FreeGroup(x):
"""
Expand Down Expand Up @@ -164,8 +165,8 @@ class FreeGroupElement(ElementLibGAP):
INPUT:
- ``x`` -- something that determines the group element. Either a
:class:`~sage.libs.gap.element.GapElement` or the Tietze list
(see :meth:`Tietze`) of the group element.
:class:`~gappy.gapobj.GapObj` or the Tietze list (see :meth:`Tietze`) of
the group element.
- ``parent`` -- the parent :class:`FreeGroup`.
Expand Down Expand Up @@ -207,7 +208,7 @@ def __init__(self, parent, x):
sage: TestSuite(G).run()
sage: TestSuite(x).run()
"""
if not isinstance(x, GapElement):
if not isinstance(x, GapObj):
try:
l = x.Tietze()
except AttributeError:
Expand Down Expand Up @@ -691,9 +692,9 @@ def wrap_FreeGroup(libgap_free_group):
``libgap_free_group`` to comparison by Python ``id``. If you want
to put the LibGAP free group into a container (set, dict) then you
should understand the implications of
:meth:`~sage.libs.gap.element.GapElement._set_compare_by_id`. To
be safe, it is recommended that you just work with the resulting
Sage :class:`FreeGroup_class`.
:meth:`~gappy.gapobj.GapObj._set_compare_by_id`. To be safe, it is
recommended that you just work with the resulting Sage
:class:`FreeGroup_class`.
INPUT:
Expand All @@ -709,7 +710,7 @@ def wrap_FreeGroup(libgap_free_group):
sage: F = libgap.FreeGroup(['a', 'b'])
sage: type(F)
<type 'sage.libs.gap.element.GapElement'>
<class 'gappy.gapobj.GapObj'>
Now wrap it::
Expand Down
6 changes: 4 additions & 2 deletions src/sage/groups/libgap_mixin.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,14 @@
"""

from sage.libs.gap.libgap import libgap
from sage.libs.gap.element import GapElement
from sage.structure.element import parent
from sage.misc.cachefunc import cached_method
from sage.groups.class_function import ClassFunction_libgap
from sage.groups.libgap_wrapper import ElementLibGAP

from gappy.gapobj import GapObj


class GroupMixinLibGAP(object):
def __contains__(self, elt):
r"""
Expand All @@ -39,7 +41,7 @@ def __contains__(self, elt):
"""
if parent(elt) is self:
return True
elif isinstance(elt, GapElement):
elif isinstance(elt, GapObj):
return elt in self.gap()
elif isinstance(elt, ElementLibGAP):
return elt.gap() in self.gap()
Expand Down
Loading

0 comments on commit 86e4b1b

Please sign in to comment.