Opened 13 months ago
Closed 12 months ago
#28965 closed enhancement (fixed)
enhance GroupLibGAP
Reported by:  vdelecroix  Owned by:  

Priority:  major  Milestone:  sage9.1 
Component:  group theory  Keywords:  
Cc:  chapoton, tscrim, sbrandhorst  Merged in:  
Authors:  Vincent Delecroix  Reviewers:  Simon Brandhorst 
Report Upstream:  N/A  Work issues:  
Branch:  74d7935 (Commits)  Commit:  74d7935b3f347cc64fa5e0f2037b14f8a233a111 
Dependencies:  Stopgaps: 
Description
 fix libgap conversion of elements
 faster group operations
 more methods
Change History (14)
comment:1 Changed 13 months ago by
 Branch set to u/vdelecroix/28965
 Commit set to 81e7ecf158613727e2fafec293f134a3dfb7a98f
 Status changed from new to needs_review
comment:2 Changed 13 months ago by
 Summary changed from enhance LibGapGroup to enhance GroupLibGAP
comment:3 followup: ↓ 4 Changed 13 months ago by
 Status changed from needs_review to needs_work
the part on class Element(ArtinGroupElement):
looks unrelated. Please move that to a different ticket.
Otherwise it looks good to me.
comment:4 in reply to: ↑ 3 Changed 13 months ago by
Replying to sbrandhorst:
the part on
class Element(ArtinGroupElement):
looks unrelated. Please move that to a different ticket. Otherwise it looks good to me.
It is related! Without this, the ArtinGroupElement
can not be rebuilt from libgap objects. Without this change, the two tests introduced in commit d90c228 just fail.
comment:5 Changed 13 months ago by
 Status changed from needs_work to positive_review
Whether or not the internal logic is okay, I do not know. The code seems to do what it says. So I am okay.
comment:6 Changed 13 months ago by
 Reviewers set to Simon Brandhorst
comment:7 Changed 13 months ago by
Thank you!
comment:8 Changed 13 months ago by
 Status changed from positive_review to needs_work
On Python 2:
********************************************************************** File "src/sage/groups/libgap_wrapper.pyx", line 505, in sage.groups.libgap_wrapper.ElementLibGAP.gap Failed example: type(libgap(FreeGroup('a, b').an_element())) Expected: <class 'sage.libs.gap.element.GapElement'> Got: <type 'sage.libs.gap.element.GapElement'> ********************************************************************** 1 item had failures: 1 of 9 in sage.groups.libgap_wrapper.ElementLibGAP.gap [141 tests, 1 failure, 0.59 s]  sage t long src/sage/groups/libgap_wrapper.pyx # 1 doctest failed 
comment:9 followup: ↓ 10 Changed 13 months ago by
Is this supposed to fail? There are tons of doctest with randomly chosen <class X>
or <type X>
!
$ git grep "<class "  wc l 1627 $ git grep "<type "  wc l 937
comment:10 in reply to: ↑ 9 Changed 13 months ago by
Replying to vdelecroix:
Is this supposed to fail? There are tons of doctest with randomly chosen
<class X>
or<type X>
!
It comes from a difference between Python2 and 3, and they are not randomly chosen. However, you can use <... X>
to make it compatible with both.
comment:11 Changed 13 months ago by
sage/groups/abelian_gps/abelian_group.py
line 961962
sage: F = AbelianGroup(3, [2], names='abc') sage: list(map(type, F.gens_orders())) [<type 'sage.rings.integer.Integer'>, <type 'sage.rings.integer.Integer'>, <type 'sage.rings.integer.Integer'>]
Which is not the output you get with Python 3. Should I just change it to type
then (it makes sage t
happy).
comment:12 Changed 13 months ago by
 Commit changed from 81e7ecf158613727e2fafec293f134a3dfb7a98f to 74d7935b3f347cc64fa5e0f2037b14f8a233a111
Branch pushed to git repo; I updated commit sha1. New commits:
74d7935  doctest compatibility with Python 2

comment:13 Changed 13 months ago by
 Status changed from needs_work to positive_review
comment:14 Changed 12 months ago by
 Branch changed from u/vdelecroix/28965 to 74d7935b3f347cc64fa5e0f2037b14f8a233a111
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
fix libgap conversion of ElementLibGAP
allow raag elements to be constructed from libgap object
rework gens of ParentLibGAP
less indirection in group operations for ElementLibGAP
clean import
more capable libgap mixin and libgap group