Opened 5 years ago

Closed 4 years ago

#16077 closed enhancement (fixed)

Python 3 preparation: Handle changes to "raw_input()" and "input()"

Reported by: wluebbe Owned by:
Priority: major Milestone: sage-6.8
Component: distribution Keywords: python3
Cc: Merged in:
Authors: André Apitzsch Reviewers: Wilfried Luebbe
Report Upstream: N/A Work issues:
Branch: ebbb545 (Commits) Commit: ebbb5451fd9880273f7138bcb517c11b68a5badb
Dependencies: Stopgaps:

Description

The Py2 function raw_input() is called input() in Py3.
The Py2 input() function (which implicitly evaluates the input) is gone.

The tool 2to3 renames raw_input to input.
But the code has to depend on the Python version!

There are 3 effected (usage of raw_input()) modules.

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

Change History (9)

comment:1 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:2 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:3 Changed 4 years ago by aapitzsch

  • Authors set to André Apitzsch
  • Branch set to u/aapitzsch/ticket/16077
  • Commit set to ebbb5451fd9880273f7138bcb517c11b68a5badb
  • Milestone changed from sage-6.4 to sage-6.8
  • Status changed from new to needs_review

comment:4 follow-ups: Changed 4 years ago by wluebbe

Instead of

+import six
...
...
-            line = raw_input()
+            line = six.moves.input()

I would prefer

+from six.moves import input
...
...
-            line = raw_input()
+            line = input()

The reasoning is

  • I see the goal as not only to enable Sage for Python 2 AND 3 but by writing clean Python 3 code that is backwards compatible with Python 2.
  • Usage of compatibility libraries like six (or future) should appear (if possible) only at the top of a module. The rest of the module should have only pure Python 3 code.
  • This will ease the cleaning of the code when somewhere in the (far) future Python 2 support will be dropped.
  • We will have to explain in the documentation how the write Python 3 code in Sage while maintaining compatibility with Python 2. And it would by very helpful to have automated checks ...

By the way all tests pass :-)

comment:5 in reply to: ↑ 4 Changed 4 years ago by aapitzsch

Replying to wluebbe:

+from six.moves import input
...
...
-            line = raw_input()
+            line = input()

would override Python2's input() which behaves different, as written in the ticket description. That's why I went the other way.

Cleaning is easy, too: s/six.moves.//g

comment:6 Changed 4 years ago by wluebbe

I did not find any uses of Python 2 input() in Sage, so there is no name conflict. This old input() has been removed from Python 3 as it is not secure because eval() is called implicitly on an unchecked string.

comment:7 Changed 4 years ago by aapitzsch

Users might import one of the effected files and therefore change the meaning of input() in their scripts. Although I doubt anybody will import these.

comment:8 in reply to: ↑ 4 Changed 4 years ago by wluebbe

  • Reviewers set to Wilfried Luebbe
  • Status changed from needs_review to positive_review

I still hold that the comment

  • I see the goal as not only to enable Sage for Python 2 AND 3 but by writing clean Python 3 code that is backwards compatible with Python 2.

is valid and important.

But this change effects only 3 modules, the code is fine and it tests OK.

But we should discuss on sage-devel how to approach the other and bigger tickets of stage 2 (usage of migration tools, which compatibility library, ...). I will try to initiate such a wider discussion of the Sage Py2/3 migration strategy. We will advance faster when there are more collaborators and ideas. A common understanding and strategy could prevent wasted efforts.

comment:9 Changed 4 years ago by vbraun

  • Branch changed from u/aapitzsch/ticket/16077 to ebbb5451fd9880273f7138bcb517c11b68a5badb
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.