Opened 9 years ago

Closed 8 years ago

#15987 closed enhancement (fixed)

Python 3 preparation: Change syntax of octal integer literals

Reported by: Wilfried Luebbe Owned by:
Priority: major Milestone: sage-6.4
Component: distribution Keywords: python3
Cc: Merged in:
Authors: Wilfried Luebbe Reviewers: Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: a17cb52 (Commits, GitHub, GitLab) Commit: a17cb523eba3d523e71d3a2c4f9b4bf90ad44ef7
Dependencies: Stopgaps:

Status badges

Description (last modified by Jeroen Demeyer)

The deprecated syntax is not accepted by Python 3.

Changes according to lib2to3/fixes/fix_numliterals.py:

0755 into 0o755

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

Attachments (2)

long-literal.warn (3.0 KB) - added by Wilfried Luebbe 9 years ago.
search result for integer literals with suffix "L" (type long)
long-literal.warn-details-both (14.5 KB) - added by Wilfried Luebbe 9 years ago.
More complete search result for long integer literals

Download all attachments as: .zip

Change History (19)

Changed 9 years ago by Wilfried Luebbe

Attachment: long-literal.warn added

search result for integer literals with suffix "L" (type long)

comment:1 Changed 9 years ago by Wilfried Luebbe

Branch: u/wluebbe/ticket/15987
Commit: a17cb523eba3d523e71d3a2c4f9b4bf90ad44ef7
Status: newneeds_review

The changes for the new octal literal format should be complete: 2 modules were fixed by the 2to3 tool and another case was manually fixed in a doc-test.

2to3 has found nothing for the long integer literal (suffix "L").
13 modules were found with regexp search (r"\s (0\d+) [\s)]"). The result file is attached.

  • 6 pyx modules from sage/rings/padics contain code like (1L << (sizeof(long) * 8 - 2)) - 1. I do not whether these should be changed.
  • 4 modules had doc-test with stuff like 4561L # 32-bit. Perhaps these test can be made more tolerant.
  • The module sage/interfaces/r.py had strings like
    "rel_re_integer.sub(a._subs_integer, ' 1L 2L')". Maybe this could / should be changed.
  • 2 modules in sage/combinat had the text "ValueError?: R10L is not a correct map". Nothing to change.

New commits:

a17cb52changed to new octal literal format (in 3 modules)

comment:2 Changed 9 years ago by Wilfried Luebbe

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:3 in reply to:  1 Changed 9 years ago by R. Andrew Ohana

Replying to wluebbe:

2to3 has found nothing for the long integer literal (suffix "L").
13 modules were found with regexp search (r"\s (0\d+) [\s)]"). The result file is attached.

  • 6 pyx modules from sage/rings/padics contain code like (1L << (sizeof(long) * 8 - 2)) - 1. I do not whether these should be changed.

These should be fine, long integer literals are important to C and hence you cannot always drop them in Cython code (even for Cython written specifically written for Python 3).

  • 4 modules had doc-test with stuff like 4561L # 32-bit. Perhaps these test can be made more tolerant.

How do you propose? The issue is that the Cython code is likely written with 64-bit integers, which on 32-bit platforms cannot always be placed into a Python2 int (since they are just a simple wrapper for the underlying platforms C long). In general you will get this issue when dealing with large integers, it is just mostly avoid in sage since we usually return Sage integers instead of Python ints or longs.

comment:4 Changed 9 years ago by Wilfried Luebbe

I was thinking about #doctest: +ELLIPSIS and replacing 4561L # 32-bit by 4561... # 32-bit.

I hope this would not run counter the purpose of the test :-)

comment:5 in reply to:  4 Changed 9 years ago by R. Andrew Ohana

Replying to wluebbe:

I was thinking about #doctest: +ELLIPSIS and replacing 4561L # 32-bit by 4561... # 32-bit.

I think ... is too liberal, e.g. the follow function would pass this doctest:

def foo():
    print("4561"+"lots of other text"*1000)

comment:6 Changed 9 years ago by Wilfried Luebbe

How about adding some magic number as a sentinel after 4561L?

E.g change

            sage: mm[1]
            10009             # 64-bit
            10009L            # 32-bit

into

            sage: mm[1], 314159
            (10009, 314159)             # 64-bit
            (10009L, 314159)            # 32-bit

comment:7 Changed 9 years ago by Wilfried Luebbe

Of course I meant (10009..., 314159) # 32-bit :-/

comment:8 Changed 9 years ago by R. Andrew Ohana

I would rather just wrapping the offending doctests with Integer and providing a comment on why the seemingly unnecessary Integer wrap is there.

comment:9 Changed 9 years ago by Wilfried Luebbe

Status: needs_reviewneeds_work

Changed 9 years ago by Wilfried Luebbe

More complete search result for long integer literals

comment:10 Changed 9 years ago by Wilfried Luebbe

The first search was incomplete. A lot more \d+L instances found (see new attachment).

I guess "fix_long" ticket:16072 (stage 2) should be solved first. That will clarify the approach to be selected here.

Move to stage 2?

comment:11 Changed 9 years ago by Wilfried Luebbe

Description: modified (diff)

comment:12 Changed 9 years ago by For batch modifications

Milestone: sage-6.2sage-6.3

comment:13 Changed 8 years ago by For batch modifications

Milestone: sage-6.3sage-6.4

comment:14 Changed 8 years ago by Jeroen Demeyer

Description: modified (diff)
Reviewers: Jeroen Demeyer
Status: needs_workpositive_review
Summary: Python 3 preparation: Change syntax of long and octal integer literalsPython 3 preparation: Change syntax of octal integer literals

comment:15 Changed 8 years ago by Volker Braun

Status: positive_reviewneeds_work

author name

comment:16 Changed 8 years ago by Jeroen Demeyer

Authors: Wilfried Luebbe
Status: needs_workpositive_review

comment:17 Changed 8 years ago by Volker Braun

Branch: u/wluebbe/ticket/15987a17cb523eba3d523e71d3a2c4f9b4bf90ad44ef7
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.