Opened 2 years ago

Closed 2 years ago

#23878 closed enhancement (fixed)

allow libgap integers as indices

Reported by: vdelecroix Owned by:
Priority: major Milestone: sage-8.1
Component: interfaces Keywords:
Cc: Merged in:
Authors: Vincent Delecroix Reviewers: Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: a102cf1 (Commits) Commit: a102cf134c5c9b355b45922a19516ea62a76e71c
Dependencies: Stopgaps:

Description

sage: s = 'abcd'
sage: s[libgap(1)]
Traceback (most recent call last):
...
TypeError: string indices must be integers,
not sage.libs.gap.element.GapElement_Integer

Change History (10)

comment:1 Changed 2 years ago by vdelecroix

  • Branch set to u/vdelecroix/23878
  • Commit set to 5bdbbb01b065b5bb4c19891b69ea7860117c56c4
  • Status changed from new to needs_review

New commits:

5bdbbb023878: allow libgap integers as indices

comment:2 Changed 2 years ago by jdemeyer

I would prefer return int(self) instead of duplicating the __int__ code.

comment:3 follow-up: Changed 2 years ago by vdelecroix

Agreed, but it ends up with a lot of indirections... (though one might hope that Cython actually bypass some of them)

comment:4 Changed 2 years ago by git

  • Commit changed from 5bdbbb01b065b5bb4c19891b69ea7860117c56c4 to a102cf134c5c9b355b45922a19516ea62a76e71c

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

a102cf123878: allow libgap integers as indices

comment:5 Changed 2 years ago by vdelecroix

done

comment:6 in reply to: ↑ 3 Changed 2 years ago by jdemeyer

Replying to vdelecroix:

Agreed, but it ends up with a lot of indirections...

True, but those indirections are not much of an issue since they are in Cython and C.

comment:7 Changed 2 years ago by jdemeyer

positive_review if it passes tests.

comment:8 Changed 2 years ago by jdemeyer

  • Reviewers set to Jeroen Demeyer

comment:9 Changed 2 years ago by jdemeyer

  • Status changed from needs_review to positive_review

comment:10 Changed 2 years ago by vbraun

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