Opened 2 years ago

Closed 2 years ago

#23704 closed enhancement (fixed)

getitem/setitem for libgap

Reported by: vdelecroix Owned by:
Priority: major Milestone: sage-8.1
Component: interfaces Keywords: days88, IMA coding sprints
Cc: tscrim Merged in:
Authors: Vincent Delecroix Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: c3241d6 (Commits) Commit: c3241d6cf5d6e93e5830c858d7a5a4cee88b14c9
Dependencies: Stopgaps:

Description

improve getitem so that the following works

sage: l = libgap.eval('[[0,1],[2,3]]')
sage: l[0,0]

and implement setitem in order to be able to do

sage: l = libgap.eval('[[0,1],[2,3]]')
sage: l[0,0] = -3

Change History (7)

comment:1 Changed 2 years ago by vdelecroix

  • Branch set to u/vdelecroix/23704
  • Commit set to 457d2fbd122ea7c9c20489cdea11f46bb8a8aed1
  • Status changed from new to needs_review

New commits:

457d2fb23704: getitem/setitem for libgap elements

comment:2 Changed 2 years ago by tscrim

Some trivial things:

Remove the period:

IndexError('index out of range.')

Minor formatting:

-        Set the i-th item of this list
+        Set the ``i``-th item of this list.

You have both multiindices and multi indices. IMO, multi-indices is best, but the biggest thing is being consistent.

Last edited 2 years ago by tscrim (previous) (diff)

comment:3 Changed 2 years ago by git

  • Commit changed from 457d2fbd122ea7c9c20489cdea11f46bb8a8aed1 to c3241d6cf5d6e93e5830c858d7a5a4cee88b14c9

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

4dc210b23704: forgot to set j appropriately!
c3241d623704: "multi-indices "multi indices" "multiindices"

comment:4 Changed 2 years ago by vdelecroix

I fixed the documentation issues in c3241d6 (though not that this documentation is not compiled in the reference manual).

More importantly, __setitem__ was not working correctly and that was addressed in 4dc210b.

comment:5 Changed 2 years ago by tscrim

  • Keywords IMA coding sprints added
  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

Thanks.

comment:6 Changed 2 years ago by vdelecroix

  • Keywords days88 added; sd88 removed

comment:7 Changed 2 years ago by vbraun

  • Branch changed from u/vdelecroix/23704 to c3241d6cf5d6e93e5830c858d7a5a4cee88b14c9
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.