py3: fix sage.libs.readline.pyx for python3

[vklein]

fix doctests failures : sage -t --long src/sage/libs/readline.pyx # 10 doctests failed

[vklein]

New commits:

​aa4cb2a

Trac #27275: Fix readline.pyx for python3

[chapoton]

seems ok at first sight.. Erik, do you approve ?

[embray]

I'm not sure. I don't think bytes_to_str should be used in doctests if it can be helped, but that's more of an arbitrary stylistic choice. Let me look at whether there's a nicer way.

[embray]

Couldn't we instead modify the copy_text function to call bytes_to_str on the return value? AFAICT this code isn't even used anywhere in Sage, but it would probably be nicer that way regardless.

[vklein]

Replying to embray:

Couldn't we instead modify the copy_text function to call bytes_to_str on the return value? AFAICT this code isn't even used anywhere in Sage, but it would probably be nicer that way regardless.

I will try that.

[git]

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

​689f9a4

Trac #27275: wrap copy_text result with a bytes_to_str ...

[vklein]

@embray I don't see drawback with your solution, let's try this.

[embray]

Just one other minor nit to pick (with the existing tests before this ticket):

src/sage/libs/readline.pyx

@@ -20 +20 @@
 
     sage: with interleaved_output():
     ....:     print('output')
-    ....:     print('current line: ' + repr(copy_text(0, get_end())))
+    ....:     print('current line: ' +
+    ....:            repr(copy_text(0, get_end())))
     ....:     print('cursor position: {}'.format(get_point()))
     output
     current line: ''

with the Python 3 print() function (which I believe we are using in most modules on Python 2 as well), there's no need for explicit string concatenation with +. The prints() in this test could be written like print('current line:', blahblahblah). I think we should promote more idiomatic code (even if in relatively minor tests that most users won't look at).

But don't bother unless you feel like it.

[embray]

Similarly the line print('cursor position: {}'.format(get_point())) could just be written simply

print('cursor position:', get_point())

[vklein]

Replying to embray:

Similarly the line print('cursor position: {}'.format(get_point())) could just be written simply

print('cursor position:', get_point())

Oh thanks ! That's good to know.

I will make these little modifications.

[git]

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

​aadc9c5

Trac #27275: Some syntax enhancement.

[embray]

Did you test this? When you do print(a, b) it automatically inserts spaces, so I think now you have an extra space.

[tscrim]

Replying to embray:

Did you test this? When you do print(a, b) it automatically inserts spaces, so I think now you have an extra space.

Be careful with print on Python2:

>>> print('a','b')
('a', 'b')

versus Python3:

>>> print('a', 'b')
a b

So I would not use the comma version in order to be Python2/3 compatible.

[vklein]

Replying to embray:

Did you test this? When you do print(a, b) it automatically inserts spaces, so I think now you have an extra space.

I think i have. But i am not sure anymore. And i don't understand why the last patchbot is green. I am currently doing more tests.

[vklein]

Well that's pretty strange even if with python2 you get that in sage console:

sage: from sage.libs.readline import *
sage: replace_line('foobar', 0)
sage: set_point(3)
sage: print('cursor position: ', get_point())
('cursor position: ', 3)

The doctest

sage: print('cursor position: ', get_point())
cursor position: 3

still pass.

And for python3 the doctest framework doesn't seem to care if there is one or two spaces between cursor position: and 3.

Even if it pass i think it's more clear and consistent to rollback to the format syntax.

[embray]

Replying to tscrim:

Replying to embray:

Did you test this? When you do print(a, b) it automatically inserts spaces, so I think now you have an extra space.

Be careful with print on Python2:

>>> print('a','b')
('a', 'b')

versus Python3:

>>> print('a', 'b')
a b

So I would not use the comma version in order to be Python2/3 compatible.

We're using from __future__ import print_function almost everywhere. If there's a module that's not using it that's a mistake.

[embray]

Replying to vklein:

Well that's pretty strange even if with python2 you get that in sage console:

sage: from sage.libs.readline import *
sage: replace_line('foobar', 0)
sage: set_point(3)
sage: print('cursor position: ', get_point())
('cursor position: ', 3)

The doctest

sage: print('cursor position: ', get_point())
cursor position: 3

still pass.

And for python3 the doctest framework doesn't seem to care if there is one or two spaces between cursor position: and 3.

I'm not sure why it is because that's wrong.

For the doctests we are using the Python 3 print() function where possible.

[embray]

See e.g. #23551.

[git]

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

[vklein]

Replying to embray:

See e.g. #23551.

Ok that explain things ! But is it a good thing to use the print('a', 'b') syntax right now ? Travis comment and mines show that it's confusing if you are not aware of #23551.

[embray]

I think for miscellaneous doctests where it's really beside the point, I think it should be fine, and better to use future-proof best practices. It's just in tutorials and other educational documentation for beginners where one needs to be a little more careful about this (or better yet, teach the difference!)

I hope we can move toward using the print function in the REPL even on Python 2 before long...

[git]

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

​f21acf8	Trac #27275: Fix readline.pyx for python3
​a291512	Trac #27275: wrap copy_text result with a bytes_to_str ...
​7ccd2a5	Trac #27275: Enhance doctest syntax.

[vklein]

Rebased on 8.7.beta4 and fix doctests with print('a', 'b') syntax with automatically inserted spaces taken into account.

[chapoton]

ok, let it be.