Opened 9 years ago

Closed 8 years ago

#15992 closed enhancement (fixed)

Python 3 preparation: Change backticks into repr()

Reported by: Wilfried Luebbe Owned by:
Priority: major Milestone: sage-6.2
Component: distribution Keywords: python3
Cc: Merged in:
Authors: Wilfried Luebbe Reviewers: Frédéric Chapoton, Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 7daa857 (Commits, GitHub, GitLab) Commit: 7daa85799b098fc9099412bd2cc095e465082c89
Dependencies: Stopgaps:

Status badges

Description

Backticks are deprecated since long ago and not accepted by Python 3.

Changes according to lib2to3/fixes/fix_repr.py:

transforms `xyzzy` into repr(xyzzy)

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

Change History (8)

comment:1 Changed 9 years ago by Wilfried Luebbe

Branch: u/wluebbe/ticket/15992
Commit: 289b3464151cfffe0dea435752141715ded7843f
Status: newneeds_review

2to3 changed 4 affected .py modules.

I have not found a way to identify the relevant backtick usages (too many backticks in comments and doc-test):

  • .pyx modules may be affected. But cython is said to be Python 3 enabled, so that should be no problem.
  • If there backticks in doc-tests they will be found during tests in a Python 3 environment.

New commits:

289b346changes the deprecated backticks usage to repr() function calls

comment:2 Changed 9 years ago by Wilfried Luebbe

Status: needs_reviewneeds_work

Apparently the automerge fails. How to locate the conflicting parts?

comment:3 Changed 9 years ago by git

Commit: 289b3464151cfffe0dea435752141715ded7843f8c99a208810f5f47b202630eb8866f4f58330d1f

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

8c99a20changes the deprecated backticks usage to repr() function calls

comment:4 Changed 9 years ago by Wilfried Luebbe

branch was rebased to resolve merge conflict

comment:5 Changed 9 years ago by Wilfried Luebbe

Status: needs_workneeds_review

I did

git merge u/chapoton/15991 u/wluebbe/ticket/15986 u/wluebbe/ticket/15987

on top of u/wluebbe/ticket/15992 (which is rebased on 6.2.beta5)

Test report:

./sage -t -p --all --long --logfile=logs/ptestlong-alle4.log
...
All tests passed!

comment:6 Changed 8 years ago by Frédéric Chapoton

Branch: u/wluebbe/ticket/15992public/ticket/15992
Commit: 8c99a208810f5f47b202630eb8866f4f58330d1f7daa85799b098fc9099412bd2cc095e465082c89
Reviewers: Frédéric Chapoton

Here is a reviewer branch. I have taken the opportunity to make small changes in the close nieghborhood of the modified code, and to add some doctests.

If you agree with my changes, you can set this to positive review.


New commits:

7daa857trac #15992 reviewer patch

comment:7 Changed 8 years ago by Travis Scrimshaw

Authors: Wilfried Luebbe
Reviewers: Frédéric ChapotonFrédéric Chapoton, Travis Scrimshaw
Status: needs_reviewpositive_review

Eeek, I'm surprised I've never noticed that not raising an error message in NCSF. I agree with the changes, so positive review.

comment:8 Changed 8 years ago by Volker Braun

Branch: public/ticket/159927daa85799b098fc9099412bd2cc095e465082c89
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.