Opened 5 years ago

Closed 5 years ago

#16750 closed defect (fixed)

libgap fails converting char GapElements to Sage

Reported by: rws Owned by:
Priority: major Milestone: sage-6.3
Component: interfaces Keywords: conversion, string
Cc: Merged in:
Authors: Volker Braun Reviewers: Ralf Stephan
Report Upstream: N/A Work issues:
Branch: 6fe1fc5 (Commits) Commit: 6fe1fc53719f295a78279635c25f581b39962912
Dependencies: Stopgaps:

Description

This blocks combinatorial usage of libgap (see #16719):

sage: ans=libgap.eval("'c'")
sage: type(ans)
<type 'sage.libs.gap.element.GapElement'>
sage: ans.sage()
---------------------------------------------------------------------------
NotImplementedError                       Traceback (most recent call last)
<ipython-input-3-3aef44f215a1> in <module>()
----> 1 ans.sage()

/home/ralf/sage/local/lib/python2.7/site-packages/sage/libs/gap/element.so in sage.libs.gap.element.GapElement.sage (build/cythonized/sage/libs/gap/element.c:8749)()

Change History (19)

comment:1 Changed 5 years ago by vbraun

To convert datatypes are supposed to use

sage: gap_string = libgap('sage_string')
sage: gap_string.sage()
'sage_string'

and not the eval() form. Though I agree that there is a bug in the eval().

comment:2 Changed 5 years ago by vbraun

The single quote is a single character constant in GAP. There is no single-character type in Python...

sage: libgap.eval("'ca'")
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-27-41441932e6b8> in <module>()
----> 1 libgap.eval("'ca'")

/home/vbraun/Code/sage/local/lib/python2.7/site-packages/sage/libs/gap/libgap.so in sage.libs.gap.libgap.Gap.eval (build/cythonized/sage/libs/gap/libgap.c:3790)()

/home/vbraun/Code/sage/local/lib/python2.7/site-packages/sage/libs/gap/util.so in sage.libs.gap.util.gap_eval (build/cythonized/sage/libs/gap/util.c:4487)()

ValueError: libGAP: Syntax error: missing single quote in character constant
'ca';
 ^

comment:3 Changed 5 years ago by vbraun

  • Branch set to u/vbraun/libgap_fails_converting_string_gapelements_to_sage

comment:4 Changed 5 years ago by vbraun

  • Authors set to Volker Braun
  • Commit set to 843ba48dc7753d79e51ea61e7e2ec3daa9149878
  • Status changed from new to needs_review

New commits:

843ba48Convert Gap chars into Python strings

comment:5 Changed 5 years ago by vbraun

  • Summary changed from libgap fails converting string GapElements to Sage to libgap fails converting char GapElements to Sage

comment:6 Changed 5 years ago by rws

  • Reviewers set to Ralf Stephan
  • Status changed from needs_review to positive_review

Looks straightforward. I do not think any other doctest is affected so I spare me a patchbot run.

comment:7 Changed 5 years ago by rws

  • Status changed from positive_review to needs_review

Holding back for just a moment, as I'm no longer sure it's ok.

comment:8 Changed 5 years ago by rws

  • Status changed from needs_review to positive_review

The sage() member function of T_CHAR will be passed to parent because no T_CHAR child is implemented.

sage: ans=libgap.eval("UnorderedTuples(['a', 'b', 'c'],2)"); ans
[ "aa", "ab", "ac", "bb", "bc", "cc" ]
sage: a=ans[0]; a
"aa"
sage: a[0]
'a'
sage: type(_)
<type 'sage.libs.gap.element.GapElement'>
sage: a[0]._type_number()
(9L, 'T_CHAR')
sage: a[0].sage()
Traceback (most recent call last):
  File "<ipython-input-50-b805c595d1a5>", line 1, in <module>
    a[Integer(0)].sage()
  File "element.pyx", line 1024, in sage.libs.gap.element.GapElement.sage (build/cythonized/sage/libs/gap/element.c:8749)
NotImplementedError: cannot construct equivalent Sage object

I have changed your patch according to my belief what went wrong. If you disagree then, well, fix it yourself 8)

comment:9 Changed 5 years ago by rws

  • Branch changed from u/vbraun/libgap_fails_converting_string_gapelements_to_sage to u/rws/libgap_fails_converting_string_gapelements_to_sage

comment:10 Changed 5 years ago by git

  • Commit changed from 843ba48dc7753d79e51ea61e7e2ec3daa9149878 to c8b583de8c985235ee6b3366d98e294209533936
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

c8b583d16750 reviewer's patch: attempt to fix previous patch

comment:11 Changed 5 years ago by rws

  • Status changed from needs_review to positive_review

comment:12 Changed 5 years ago by vbraun

  • Branch changed from u/rws/libgap_fails_converting_string_gapelements_to_sage to u/vbraun/libgap_fails_converting_string_gapelements_to_sage

comment:13 Changed 5 years ago by vbraun

  • Commit changed from c8b583de8c985235ee6b3366d98e294209533936 to 7cc26d56d116f0fe976a031948c49b77827aa9e6
  • Status changed from positive_review to needs_review

I read some more of the GAP String code and its pretty complicated with different string representations. I've replaced your commit. This also fixes the string convertion issue you noted on the other ticket. Can you give it at try?


New commits:

7cc26d5Improve GAP String handling

comment:14 Changed 5 years ago by rws

ptestlong:

sage -t --long src/sage/groups/braid.py  # 5 doctests failed
sage -t --long src/sage/groups/finitely_presented.py  # 25 doctests failed
sage -t --long src/sage/groups/finitely_presented_named.py  # 4 doctests failed
sage -t --long src/sage/homology/simplicial_complex.py  # 3 doctests failed
sage -t --long src/sage/groups/libgap_mixin.py  # 1 doctest failed
sage -t --long src/sage/groups/perm_gps/partn_ref2/refinement_generic.pyx  # 1 doctest failed
sage -t --long src/sage/groups/raag.py  # 16 doctests failed
sage -t --long src/sage/groups/libgap_wrapper.pyx  # 1 doctest failed

Appears to go back to a single fail.

comment:15 Changed 5 years ago by git

  • Commit changed from 7cc26d56d116f0fe976a031948c49b77827aa9e6 to 6fe1fc53719f295a78279635c25f581b39962912

Branch pushed to git repo; I updated commit sha1. New commits:

6fe1fc5Treat the empty GAP list as list and not as string

comment:16 Changed 5 years ago by rws

Fails in new files:

sage -t --long src/sage/groups/perm_gps/permgroup.py  # 2 doctests failed
sage -t --long src/sage/groups/perm_gps/permgroup_element.pyx  # 2 doctests failed

comment:17 Changed 5 years ago by vbraun

It does? I remembered to run tests this time and it works for me. Can you post the error?

comment:18 Changed 5 years ago by rws

  • Status changed from needs_review to positive_review

Don't know what *that was, it passes now after checking out a different branch, compiling, and back again.

Anyway, it all looks good.

comment:19 Changed 5 years ago by vbraun

  • Branch changed from u/vbraun/libgap_fails_converting_string_gapelements_to_sage to 6fe1fc53719f295a78279635c25f581b39962912
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.