py3 print : not tested cases, step 5

[fchapoton]

another small step towards python3

some more print converted to use python3 syntax

CC: @tscrim

Component: python3

Author: Frédéric Chapoton

Branch/Commit: fc51d8e

Reviewer: Jori Mäntysalo, Travis Scrimshaw

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

[fchapoton]

Branch: public/20814

[fchapoton]

Commit: fd1af37

[fchapoton]

New commits:

5ef5316	more print converted to python3 in py files
fd1af37	more print converted to py3 in .py files

[jm58660]

comment:2

An error here:

-    if verbose: print "Check:",
+    if verbose:
+        print("Check:", end="")

needs a space after colon.

[jm58660]

Reviewer: Jori Mäntysalo

[jm58660]

comment:3

I found no other errors. You can mark this as positive_review after correcting above error.

This is probably my last review for a week. Btw, if you have much time you might be interested in reviewing the code at #20769, or if you have some minutes, making a comment to #20669.

[sagetrac-git]

Changed commit from fd1af37 to 93e0bb5

[sagetrac-git]

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

93e0bb5

trac 20814 detail

[vbraun]

comment:6

Merge conflict, wait for next beta

[sagetrac-git]

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

08967ef

Merge branch 'public/20814' into 7.3.b4

[sagetrac-git]

Changed commit from 93e0bb5 to 08967ef

[sagetrac-git]

Changed commit from 08967ef to a7146ea

[sagetrac-git]

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

a7146ea

correct or disactivate the last python2-only print doctests

[jm58660]

comment:12

No time now. Travis?

[fchapoton]

comment:13

bot is green

[tscrim]

comment:14

So with this, every time from this point forward we can never have a Python2 print statement in the doctests? Do I understand the ticket description correctly?

[fchapoton]

comment:15

Replying to @tscrim:

So with this, every time from this point forward we can never have a Python2 print statement in the doctests? Do I understand the ticket description correctly?

Yes, I think so. this seems to be triggered by the change in structure/test_factory.py.
I was not expecting that, but as this was something I wanted to do anyway, I am happy
to do it here.

EDIT: I do not understand by what change this is triggered. Maybe one in misc ?

1. I have not tested the new behaviour by adding a fake file with a bad print
2. one should also check that the print behaviour in console and notebooks is still py2
3. maybe this should be discussed on sage-devel, though this is not a major change really
4. maybe this can/should be split in another ticket ?

This is the keystone of all my previous changes in the doc, that prevents any regression.

[fchapoton]

comment:16

I have tested by adding a fake new file with a bad print in the doc: tests do not pass

I have checked that the behaviour of print is unchanged in console and both notebooks.

I still do not understand why this desirable behaviour happens.

[fchapoton]

comment:17

ping ?

[tscrim]

comment:18

I think it was the change in misc/dev_tools.py. While in principle, I am for this change, I do believe we should have some discussion on sage-devel about it. In general, we can write 2/3 compatible docstrings, but we cannot strictly enforce it. I am in favor of enforcing the Python3 syntax, but it does create a discrepancy (at least if we add a strictly Python3 print statement). If there are no objections on sage-devel, then I am content with putting this to a positive review.

[fchapoton]

comment:19

I do not think that it comes from the changes in misc/dev_tools.py

I have really tried to see where the change of behaviour came from, with no success so far.

I would now prefer this ticket not to make this change of behaviour, but for this I need to understand what has to be undone, and I have not clue.

[sagetrac-git]

Changed commit from a7146ea to d832810

[sagetrac-git]

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

d832810

trac 20814 restore the exact file where future import is not yet welcome

[fchapoton]

comment:21

ok, I think I have found that the problems were coming from the future import in src/sage/modular/etaproducts.py

No idea what happens exactly, but undoing this change gives us back the usual behaviour.

So this is back to being a very simple print ticket, one of the last ones for py files.

[tscrim]

comment:22

If you could remove the added space in the copyright header for consistency, then you can set a positive review on my behalf.

[tscrim]

Changed reviewer from Jori Mäntysalo to Jori Mäntysalo, Travis Scrimshaw

[fchapoton]

comment:23

The added space remove I can (Yoda mode), but this change is suggested by pep8. Should I remove ?

[tscrim]

comment:24

I think so. We consistently break the PEP8 in this case, and in our doc about file headers, the copyright is given without a space as well.

[sagetrac-git]

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

fc51d8e

tarc 20814 not pep8 for copyright header

[sagetrac-git]

Changed commit from d832810 to fc51d8e

[tscrim]

comment:26

Thanks.

[vbraun]

Changed branch from public/20814 to fc51d8e