Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle conversion to infinity and int in libgap #18153

Closed
videlec opened this issue Apr 9, 2015 · 21 comments
Closed

Handle conversion to infinity and int in libgap #18153

videlec opened this issue Apr 9, 2015 · 21 comments

Comments

@videlec
Copy link
Contributor

videlec commented Apr 9, 2015

We just add support to the following

sage: int(libgap(3))
3
sage: libgap(Infinity).sage()
+Infinity
sage: libgap(-Infinity).sage()
-Infinity
sage: libgap.one()
1

CC: @vbraun

Component: interfaces

Author: Vincent Delecroix

Branch/Commit: cc3297d

Reviewer: Volker Braun

Issue created by migration from https://trac.sagemath.org/ticket/18153

@videlec videlec added this to the sage-6.6 milestone Apr 9, 2015
@videlec
Copy link
Contributor Author

videlec commented Apr 9, 2015

New commits:

31385f3Trac 18153: int and infinity from libgap

@videlec
Copy link
Contributor Author

videlec commented Apr 9, 2015

Commit: 31385f3

@videlec
Copy link
Contributor Author

videlec commented Apr 9, 2015

Branch: u/vdelecroix/18153

@videlec

This comment has been minimized.

@vbraun
Copy link
Member

vbraun commented Apr 10, 2015

comment:3

DRY: __int__ is just self.sage(ring=int).

There is also IsNegInfinity for negative infinity...

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 10, 2015

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

4ec9f5fTrac 18153: better `__int__` and add support for -infinity

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 10, 2015

Changed commit from 31385f3 to 4ec9f5f

@videlec

This comment has been minimized.

@vbraun
Copy link
Member

vbraun commented Apr 10, 2015

comment:6

How about also adding the other way, libgap(oo) should do the right thing.... e.g. by adding a _libgap_ method to sage.rings.infinity.PlusInfinity. Then you could also clean up the docstrings, the raw libgap.eval isn't what you would want to show to users.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 10, 2015

Changed commit from 4ec9f5f to cdcc7d2

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 10, 2015

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

cdcc7d2Trac 18153: sage to gap conversion for infinity

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 10, 2015

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

57c6685Trac 18153: sage to gap conversion for infinity

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 10, 2015

Changed commit from cdcc7d2 to 57c6685

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 10, 2015

Changed commit from 57c6685 to cc3297d

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 10, 2015

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

cc3297dTrac 18153: sage to gap conversion for infinity

@videlec
Copy link
Contributor Author

videlec commented Apr 10, 2015

comment:10

Replying to @vbraun:

How about also adding the other way, libgap(oo) should do the right thing.... e.g. by adding a _libgap_ method to sage.rings.infinity.PlusInfinity. Then you could also clean up the docstrings, the raw libgap.eval isn't what you would want to show to users.

Right! I did implement a _gap_init_ method. That way even the gap interface gets some benefit in it.

Vincent

@videlec

This comment has been minimized.

@vbraun
Copy link
Member

vbraun commented Apr 10, 2015

Reviewer: Volker Braun

@vbraun
Copy link
Member

vbraun commented Apr 10, 2015

comment:12

Thanks, lgtm!

@videlec
Copy link
Contributor Author

videlec commented Apr 10, 2015

comment:13

Thanks for the review.

@vbraun
Copy link
Member

vbraun commented Apr 14, 2015

Changed branch from u/vdelecroix/18153 to cc3297d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants