Opened 13 months ago
Closed 12 months ago
#28965 closed enhancement (fixed)
enhance GroupLibGAP
Reported by: | vdelecroix | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-9.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 follow-up: ↓ 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 follow-up: ↓ 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 961-962
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