Opened 8 years ago
Closed 8 years ago
#15987 closed enhancement (fixed)
Python 3 preparation: Change syntax of octal integer literals
Reported by: | wluebbe | 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: |
Description (last modified by )
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)
Change History (19)
Changed 8 years ago by
comment:1 follow-up: ↓ 3 Changed 8 years ago by
- Branch set to u/wluebbe/ticket/15987
- Commit set to a17cb523eba3d523e71d3a2c4f9b4bf90ad44ef7
- Status changed from new to needs_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:
a17cb52 | changed to new octal literal format (in 3 modules)
|
comment:2 Changed 8 years ago by
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 8 years ago by
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 follow-up: ↓ 5 Changed 8 years ago by
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 8 years ago by
Replying to wluebbe:
I was thinking about
#doctest: +ELLIPSIS
and replacing4561L # 32-bit
by4561... # 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 8 years ago by
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 8 years ago by
Of course I meant (10009..., 314159) # 32-bit
:-/
comment:8 Changed 8 years ago by
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 8 years ago by
- Status changed from needs_review to needs_work
comment:10 Changed 8 years ago by
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 8 years ago by
- Description modified (diff)
comment:12 Changed 8 years ago by
- Milestone changed from sage-6.2 to sage-6.3
comment:13 Changed 8 years ago by
- Milestone changed from sage-6.3 to sage-6.4
comment:14 Changed 8 years ago by
- Description modified (diff)
- Reviewers set to Jeroen Demeyer
- Status changed from needs_work to positive_review
- Summary changed from Python 3 preparation: Change syntax of long and octal integer literals to Python 3 preparation: Change syntax of octal integer literals
comment:16 Changed 8 years ago by
- Status changed from needs_work to positive_review
comment:17 Changed 8 years ago by
- Branch changed from u/wluebbe/ticket/15987 to a17cb523eba3d523e71d3a2c4f9b4bf90ad44ef7
- Resolution set to fixed
- Status changed from positive_review to closed
search result for integer literals with suffix "L" (type long)