Opened 7 years ago

Closed 4 years ago

#15989 closed enhancement (fixed)

Python 3 preparation: Change print statement to print() function

Reported by: wluebbe Owned by:
Priority: major Milestone: sage-7.3
Component: python3 Keywords: python3
Cc: jdemeyer, tscrim, jmantysalo Merged in:
Authors: Frédéric Chapoton Reviewers: Jori Mäntysalo
Report Upstream: N/A Work issues:
Branch: b3a6ebc (Commits) Commit: b3a6ebcee04f6d7520c372aa611bde74fa307983
Dependencies: Stopgaps:

Description

To be Python 2 compatible all affected modules need a from __future__ import print_function.

lib2to3/fixes/fix_import.py does not add these __future__ imports.
Therefore we intend to use an enhanced fixer from https://pypi.python.org/pypi/future/0.11.3.

Changes according to libfuturize/fixes/fix_print_with_import.py:

Change:
    'print'          into 'print()'
    'print ...'      into 'print(...)'
    'print ... ,'    into 'print(..., end=" ")'
    'print >>x, ...' into 'print(..., file=x)'
No changes are applied if print_function is imported from __future__
-------- 
Turns any print statements into functions and adds this import line:
    from __future__ import print_function
at the top to retain compatibility with Python 2.6+.

This ticket is tracked as a dependency of meta-ticket ticket:15980.

Change History (20)

comment:1 Changed 6 years ago by wluebbe

In Sage there are currently only 4 py modules with an from __future__ import print_function.

From the Python documentation:

Note: This function <the print() function> is not normally available as a built-in since the name print is recognized as the print statement. To disable the statement and use the print() function, use this future statement at the top of your module: "from __future__ import print_function". New in version 2.6.

There are about 50 py modules that contain uses of the print statement that look like a function call (i.e. having parenthesis). In some cases 2to3 will wrap an additional pair of parenthesis around it. But as the following snippet shows this should be neither in Py2.7 nor in Py3.3 a problem:

Python 2.7.5+ (default, Feb 27 2014, 19:37:08) 

>>> print "text"
text
>>> print"text"
text
>>> print("text")
text
>>> print(("text"))
text

>>> from __future__ import print_function
>>> print"text"
  File "<stdin>", line 1
    print"text"
              ^
SyntaxError: invalid syntax
>>> print "text"
  File "<stdin>", line 1
    print "text"
               ^
SyntaxError: invalid syntax
>>> print("text")
text
>>> print(("text"))
text
>>> 

comment:2 Changed 6 years ago by wluebbe

A discussion in ticket:13255 lead to the idea to NOT include from __future__ import print_function at the top of modules but to wrap a pair of parenthesis around the parameter(s) of the print statement. This should also satisfy the print function of Python 3.

For testing the completeness from __future__ import print_function might be added to some module that is (almost) universally imported.

Are there any drawbacks?

comment:3 Changed 6 years ago by ohanar

This will give unexpected results with a trailing comma, or a redirected output. If either these cases appear in a file (and I'm almost positive the first does appear in the sage library), I see no way to get around importing future for these modules.

comment:4 Changed 6 years ago by wluebbe

I agree. I will do some searches soon.

comment:5 Changed 6 years ago by wluebbe

In py modules there are

  • 41 matches for end= (or even , end=' '))
  • 1 match for file= (more precise file=sys.stderr)) in src/sage/all.py.

comment:6 Changed 6 years ago by wluebbe

I took the following approach:

  1. Run futurize -f print_with_import.py. This changed about 200 py modules.
  2. Run the doctests. There were failures in about 480 .py/.rst-files. The file with the failures had almost 40.000 lines.
  3. I started (slowly) to fix the doctests by wrapping the print parameters in parenthesis.

But the I began to doubt whether this is the right thing to do:

Now I think it would better to

  • First enable the preparser to change the print statement syntax into the function syntax whenever "print" refers to '<built-in function print>' (which is the case in Py3 but also in Py2 when from __future__ import print_function is in effect).
  • Then add parenthesis to the print statements in the py modules (adding from __future__ import is optional).
  • If required (because of ">>>file" or a trailing comma) use the new function syntax with from __future__ import print_function.
  • Doctest should by changed if and when considered reasonable.
Last edited 6 years ago by ohanar (previous) (diff)

comment:7 follow-up: Changed 6 years ago by ohanar

Yes, I tend to agree that is a better approach. I have been working on rewriting the preparser to use the tokenize module as much as possible (as well as refactor some stuff), but I'm pretty busy this month, so I don't know when exactly I'll be done with that. It should be very simple to add this functionality once I'm done however.

comment:8 Changed 6 years ago by wluebbe

I made another attempt using 2to3 -f print: this uses no from __future__ import print_function but changes the print statement syntax to the print function syntax.
The hope was that the majority of changes would also be valid statement syntax (just an extra pair of parenthesis). See ticket:13255 for such an approach.

There were about 200 changed py modules.
Thereof about 20 had lines with end=" ". Some could be eliminated with a little refactoring.

14 modules needed import print_function. This lead usually 40 doctest failures as the parenthesis caused the parameters to be printed as tuples (which was not the expected output). A fix would be simple.

But in src/sage/modular/etaproducts.py and in src/sage/modular/all.py (which contained the only file= the import print_function caused 2545 doctest failures in about 500 modules!!! I still cannot explain this differing behavior.

To long didn't read: I consider this experiment failed!
If the goal is one robust common Python 2 / 3 code base (which I assume!) then all modules should have the from __future__ import print_function. This would require a preparser that can add the parenthesis in the doctests on the fly or -as a fallback- add the parenthesis to the doctest source.

comment:9 Changed 6 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:10 Changed 6 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:11 in reply to: ↑ 7 ; follow-up: Changed 6 years ago by aapitzsch

Replying to ohanar:

Yes, I tend to agree that is a better approach. I have been working on rewriting the preparser to use the tokenize module as much as possible (as well as refactor some stuff), but I'm pretty busy this month, so I don't know when exactly I'll be done with that. It should be very simple to add this functionality once I'm done however.

Any progress on the preparser front?

comment:12 in reply to: ↑ 11 Changed 6 years ago by ohanar

Replying to aapitzsch:

Replying to ohanar:

Yes, I tend to agree that is a better approach. I have been working on rewriting the preparser to use the tokenize module as much as possible (as well as refactor some stuff), but I'm pretty busy this month, so I don't know when exactly I'll be done with that. It should be very simple to add this functionality once I'm done however.

Any progress on the preparser front?

No sorry, haven't had the time. Might just do a hack for now.

comment:13 Changed 4 years ago by jdemeyer

  • Component changed from distribution to python3

comment:14 Changed 4 years ago by chapoton

  • Authors set to Frédéric Chapoton
  • Branch set to public/15989
  • Cc jdemeyer tscrim jmantysalo added
  • Commit set to 5f1e4dac9b15acec900682525e391a40ecaec471
  • Milestone changed from sage-6.4 to sage-7.3
  • Status changed from new to needs_review

Here is a branch converting the last few instances of old-style print function to python3 compatible syntax.


New commits:

5f1e4daone last bunch of old-style print

comment:15 Changed 4 years ago by jmantysalo

Fails for me:

[sagelib-7.3.beta6] Compiling sage/rings/finite_rings/element_ntl_gf2e.pyx because it changed.
[sagelib-7.3.beta6] [1/1] Cythonizing sage/rings/finite_rings/element_ntl_gf2e.pyx
[sagelib-7.3.beta6] 
[sagelib-7.3.beta6] Error compiling Cython file:
[sagelib-7.3.beta6] ------------------------------------------------------------
[sagelib-7.3.beta6] ...
[sagelib-7.3.beta6]         # Set the modulus.
[sagelib-7.3.beta6]         self.F.restore()
[sagelib-7.3.beta6]         # Print the current modulus.
[sagelib-7.3.beta6]         cdef GF2XModulus_c modulus = GF2E_modulus()
[sagelib-7.3.beta6]         cdef GF2X_c mod_poly = GF2XModulus_GF2X(modulus)
[sagelib-7.3.beta6]         print(GF2X_to_PyString(&mod_poly))
[sagelib-7.3.beta6]                              ^
[sagelib-7.3.beta6] ------------------------------------------------------------
[sagelib-7.3.beta6] 
[sagelib-7.3.beta6] sage/rings/finite_rings/element_ntl_gf2e.pyx:220:30: Cannot convert Python object to 'GEN'

comment:16 Changed 4 years ago by jmantysalo

  • Status changed from needs_review to needs_work

comment:17 Changed 4 years ago by git

  • Commit changed from 5f1e4dac9b15acec900682525e391a40ecaec471 to b3a6ebcee04f6d7520c372aa611bde74fa307983

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

b3a6ebctrac 15989 remove a future_import in order to prevent build failure

comment:18 follow-up: Changed 4 years ago by chapoton

  • Status changed from needs_work to needs_review

should be better now. Thank you Jori for reviewing all these tickets !

comment:19 in reply to: ↑ 18 Changed 4 years ago by jmantysalo

  • Reviewers set to Jori Mäntysalo
  • Status changed from needs_review to positive_review

Replying to chapoton:

should be better now. Thank you Jori for reviewing all these tickets !

Now it works. Thanks for thanking.

comment:20 Changed 4 years ago by vbraun

  • Branch changed from public/15989 to b3a6ebcee04f6d7520c372aa611bde74fa307983
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.