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

Conversion PARI t_STR -> Python str should not keep quotes #19575

Closed
jdemeyer opened this issue Nov 12, 2015 · 9 comments
Closed

Conversion PARI t_STR -> Python str should not keep quotes #19575

jdemeyer opened this issue Nov 12, 2015 · 9 comments

Comments

@jdemeyer
Copy link

This should be without quotes, similar to the difference between repr() and str() of a Python string:

sage: print pari('"hello world"')
"hello world"

CC: @pjbruin

Component: interfaces: optional

Author: Jeroen Demeyer

Branch/Commit: 1668133

Reviewer: Peter Bruin

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

@jdemeyer
Copy link
Author

Branch: u/jdemeyer/str_pari_gen

@jdemeyer
Copy link
Author

New commits:

29128c2Implement conversion PARI t_STR -> Python string

@jdemeyer
Copy link
Author

Commit: 29128c2

@pjbruin
Copy link
Contributor

pjbruin commented Nov 12, 2015

comment:3
  • The patchbot reports a doctest failure in sage.interfaces.interface.InterfaceElement.__reduce__. I think the quoted version "abc" is the correct output. Probably gen.__reduce__() should use __repr__() instead of __str__().
  • It would look better to use %r in the error message that you change in the patch, and actually I would also prefer putting a space after the =, i.e.
raise TypeError("x (= %r) must be of type t_INT, but is of type %s" % (x, x.type()))

(For the second point, maybe we can even remove the type check and rely on PARI to raise an error.)

@pjbruin
Copy link
Contributor

pjbruin commented Nov 12, 2015

Reviewer: Peter Bruin

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 12, 2015

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

674fb90Use repr() for pickling
1668133Do not check type in binary(), let PARI raise an error

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 12, 2015

Changed commit from 29128c2 to 1668133

@pjbruin
Copy link
Contributor

pjbruin commented Nov 12, 2015

comment:6

Looks good.

@vbraun
Copy link
Member

vbraun commented Nov 16, 2015

Changed branch from u/jdemeyer/str_pari_gen to 1668133

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

3 participants