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:  sage6.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) <ipythoninput33aef44f215a1> in <module>() > 1 ans.sage() /home/ralf/sage/local/lib/python2.7/sitepackages/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
comment:2 Changed 5 years ago by
The single quote is a single character constant in GAP. There is no singlecharacter type in Python...
sage: libgap.eval("'ca'")  ValueError Traceback (most recent call last) <ipythoninput2741441932e6b8> in <module>() > 1 libgap.eval("'ca'") /home/vbraun/Code/sage/local/lib/python2.7/sitepackages/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/sitepackages/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
 Branch set to u/vbraun/libgap_fails_converting_string_gapelements_to_sage
comment:4 Changed 5 years ago by
 Commit set to 843ba48dc7753d79e51ea61e7e2ec3daa9149878
 Status changed from new to needs_review
New commits:
843ba48  Convert Gap chars into Python strings

comment:5 Changed 5 years ago by
 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
 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
 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
 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 "<ipythoninput50b805c595d1a5>", 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
 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
 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:
c8b583d  16750 reviewer's patch: attempt to fix previous patch

comment:11 Changed 5 years ago by
 Status changed from needs_review to positive_review
comment:12 Changed 5 years ago by
 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
 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:
7cc26d5  Improve GAP String handling

comment:14 Changed 5 years ago by
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
 Commit changed from 7cc26d56d116f0fe976a031948c49b77827aa9e6 to 6fe1fc53719f295a78279635c25f581b39962912
Branch pushed to git repo; I updated commit sha1. New commits:
6fe1fc5  Treat the empty GAP list as list and not as string

comment:16 Changed 5 years ago by
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
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
 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
 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
To convert datatypes are supposed to use
and not the eval() form. Though I agree that there is a bug in the eval().