Skip to content

Commit

Permalink
Replace most use of internal functions for strings with the public
Browse files Browse the repository at this point in the history
libgap API.  Refs #2.

The outlying exception currently is C_NEW_STRING which does not have an
exact analogue in the public libgap API.  The closest thing is
GAP_MakeString, but this assumes the string is NULL-terminated, meaning
it can't be used to convert arbitrary Python bytes objects containing
nulls.  In practice this won't often come up, but better safe than sorry
until a new API can be added.
  • Loading branch information
embray committed Jan 12, 2021
1 parent 35c04c0 commit f284a12
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 32 deletions.
2 changes: 1 addition & 1 deletion gappy/core.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,7 @@ cdef str extract_libgap_errout():

# Grab a pointer to the C string underlying the GAP string libgap_errout
# then copy it to a Python str (char_to_str contains an implicit strcpy)
msg = CSTR_STRING(r)
msg = GAP_CSTR_STRING(r)
if msg != NULL:
msg_py = char_to_str(msg)
msg_py = msg_py.replace('For debugging hints type ?Recovery from '
Expand Down
6 changes: 3 additions & 3 deletions gappy/element.pxd
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ cdef GapBoolean make_GapBoolean(parent, Obj obj)
cdef GapFunction make_GapFunction(parent, Obj obj)
cdef GapPermutation make_GapPermutation(parent, Obj obj)

cdef char *capture_stdout(Obj, Obj)
cdef char *gap_element_str(Obj)
cdef char *gap_element_repr(Obj)
cdef void capture_stdout(Obj, Obj, Obj)
cdef void gap_element_str(Obj, Obj)
cdef void gap_element_repr(Obj, Obj)


cdef class GapObj:
Expand Down
58 changes: 34 additions & 24 deletions gappy/element.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -149,15 +149,15 @@ cdef Obj make_gap_matrix(parent, lst, gap_ring) except NULL:
return l.value


cdef char *capture_stdout(Obj func, Obj obj):
cdef void capture_stdout(Obj func, Obj obj, Obj out):
"""
Call a single-argument GAP function ``func`` with the argument ``obj``
and return the stdout from that function call.
and return the stdout from that function call to the GAP string ``out``.
This can be used to capture the output of GAP functions that are used to
print objects such as ``Print()`` and ``ViewObj()``.
"""
cdef Obj s, stream, output_text_string
cdef Obj stream, output_text_string
cdef UInt res
cdef Obj args[2]
# The only way to get a string representation of an object that is truly
Expand All @@ -171,9 +171,8 @@ cdef char *capture_stdout(Obj func, Obj obj):
# support from GAP to improve this...
try:
GAP_Enter()
s = NEW_STRING(0)
output_text_string = GAP_ValueGlobalVariable("OutputTextString")
args[0] = s
args[0] = out
args[1] = GAP_True
stream = GAP_CallFuncArray(output_text_string, 2, args)

Expand All @@ -184,12 +183,11 @@ cdef char *capture_stdout(Obj func, Obj obj):
args[0] = obj
GAP_CallFuncArray(func, 1, args)
CloseOutput()
return CSTR_STRING(s)
finally:
GAP_Leave()


cdef char *gap_element_repr(Obj obj):
cdef void gap_element_repr(Obj obj, Obj out):
"""
Implement ``repr()`` of ``GapObj``s using the ``ViewObj()`` function,
which is by default closest to what you get when displaying an object in
Expand All @@ -198,10 +196,10 @@ cdef char *gap_element_repr(Obj obj):
"""

cdef Obj func = GAP_ValueGlobalVariable("ViewObj")
return capture_stdout(func, obj)
capture_stdout(func, obj, out)


cdef char *gap_element_str(Obj obj):
cdef void gap_element_str(Obj obj, Obj out):
"""
Implement ``str()`` of ``GapObj``s using the ``Print()`` function.
Expand All @@ -212,7 +210,7 @@ cdef char *gap_element_str(Obj obj):
difference (though this does not map perfectly onto GAP).
"""
cdef Obj func = GAP_ValueGlobalVariable("Print")
return capture_stdout(func, obj)
capture_stdout(func, obj, out)


cdef Obj make_gap_record(parent, dct) except NULL:
Expand Down Expand Up @@ -420,11 +418,7 @@ cdef GapObj make_any_gap_element(parent, Obj obj):
return make_GapPermutation(parent, obj)
elif IS_REC(obj):
return make_GapRecord(parent, obj)
elif GAP_IsList(obj) and GAP_LenList(obj) == 0:
# Empty lists are lists and not strings in Python
return make_GapList(parent, obj)
elif IsStringConv(obj):
# GAP strings are lists, too. Make sure this comes before non-empty make_GapList
elif GAP_IsString(obj):
return make_GapString(parent, obj)
elif GAP_IsList(obj):
return make_GapList(parent, obj)
Expand Down Expand Up @@ -856,11 +850,19 @@ cdef class GapObj:
>>> gap(0).__str__()
'0'
"""
if self.value == NULL:
cdef Obj out

if self.value == NULL:
return 'NULL'

s = char_to_str(gap_element_str(self.value))
return s.strip()
try:
GAP_Enter()
out = GAP_MakeString("")
gap_element_str(self.value, out)
s = char_to_str(GAP_CSTR_STRING(out))
return s.strip()
finally:
GAP_Leave()

def __repr__(self):
r"""
Expand All @@ -878,11 +880,19 @@ cdef class GapObj:
>>> gap(0).__repr__()
'0'
"""
if self.value == NULL:
cdef Obj out

if self.value == NULL:
return 'NULL'

s = char_to_str(gap_element_repr(self.value))
return s.strip()
try:
GAP_Enter()
out = GAP_MakeString("")
gap_element_repr(self.value, out)
s = char_to_str(GAP_CSTR_STRING(out))
return s.strip()
finally:
GAP_Leave()

cpdef _set_compare_by_id(self):
"""
Expand Down Expand Up @@ -1434,7 +1444,7 @@ cdef class GapObj:
>>> gap('this is a string').is_string()
True
"""
return IS_STRING(self.value)
return bool(GAP_IsString(self.value))

def is_permutation(self):
r"""
Expand Down Expand Up @@ -1994,7 +2004,7 @@ cdef class GapString(GapObj):
>>> type(_)
<class 'str'>
"""
s = char_to_str(CSTR_STRING(self.value))
s = char_to_str(GAP_CSTR_STRING(self.value))
return s


Expand Down Expand Up @@ -2864,7 +2874,7 @@ cdef class GapRecordIterator(object):
raise StopIteration
# note the abs: negative values mean the rec keys are not sorted
key_index = abs(GET_RNAM_PREC(self.rec.value, i))
key = char_to_str(CSTR_STRING(NAME_RNAM(key_index)))
key = char_to_str(GAP_CSTR_STRING(NAME_RNAM(key_index)))
cdef Obj result = GET_ELM_PREC(self.rec.value,i)
val = make_any_gap_element(self.rec.parent(), result)
self.i += 1
Expand Down
6 changes: 2 additions & 4 deletions gappy/gap_includes.pxd
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,9 @@ cdef extern from "gap/libgap-api.h" nogil:

# Strings
cdef char *GAP_CSTR_STRING(Obj)
cdef int GAP_IsString(Obj)
cdef UInt GAP_LenString(Obj)
cdef Obj GAP_MakeString(const char *)

# Lists
void GAP_AssList(Obj, UInt, Obj val)
Expand Down Expand Up @@ -175,8 +177,4 @@ cdef extern from "gap/records.h" nogil:


cdef extern from "gap/stringobj.h" nogil:
char* CSTR_STRING(Obj list)
bint IS_STRING(Obj obj)
bint IsStringConv(Obj obj)
Obj NEW_STRING(Int)
void C_NEW_STRING(Obj new_gap_string, int length, char* c_string)

0 comments on commit f284a12

Please sign in to comment.