Opened 7 years ago
Closed 4 years ago
#16409 closed enhancement (fixed)
2to3 refactoring tool has warnings for some code
Reported by:  wluebbe  Owned by:  

Priority:  minor  Milestone:  sage8.0 
Component:  python3  Keywords:  python3 
Cc:  tscrim, jdemeyer, jhpalmieri  Merged in:  
Authors:  Frédéric Chapoton  Reviewers:  André Apitzsch 
Report Upstream:  N/A  Work issues:  
Branch:  ba2fdb1 (Commits, GitHub, GitLab)  Commit:  ba2fdb126734c6c7b603e2d051abc0e96b93bf22 
Dependencies:  Stopgaps: 
Description
While running the Python 2to3 on the Sage code base some warning are emitted:
 fixer= absolute_import
### In file sage/misc/sage_timeit.py ### Line 199: absolute and local imports together
 fixer= map
### In file sage/geometry/polyhedron/constructor.py ### Line 364: You should use a for loop here ### In file sage/geometry/polyhedron/ppl_lattice_polytope.py ### Line 1078: You should use a for loop here
 fixer= next
### In file sage/combinat/integer_list.py ### Line 322: Calls to builtin next() possibly shadowed by global binding
 fixer= operator
### In file sage/combinat/sf/sfa.py ### Line 3520: You should use 'operator.mul(lcm(pair), times=gcd(pair))' here. ### In file sage/combinat/species/generating_series.py ### Line 724: You should use 'operator.mul(lcm(pair), times=gcd(pair))' here.
Change History (12)
comment:1 Changed 7 years ago by
 Branch set to u/wluebbe/ticket/16409
 Cc tscrim added
 Commit set to 3bd3bc57abb84722581833452ecc8943f2641fb5
 Status changed from new to needs_info
comment:2 Changed 7 years ago by
Shortterm solution would be to rename next
to next_object
or something like this (it's also at least used in combinat/integer_vector.py
). The true deprecation solution is going to be much harder (I can try to do this if you want, but it would take a lot longer to do).
comment:3 Changed 7 years ago by
But isn't {{{next()}} an element if the public API? So renaming would be no option.
I'm currently thinking it would be best to ignore this warning now.
comment:4 Changed 7 years ago by
It's not in the public (Sage) namespace, so we can rename it without having to deprecate it. However if you think it's fine to just ignore it, then that's fine (and I'll do the refactoring later this year).
comment:5 Changed 7 years ago by
 Milestone changed from sage6.3 to sage6.4
comment:6 Changed 5 years ago by
 Branch changed from u/wluebbe/ticket/16409 to public/16409
 Commit changed from 3bd3bc57abb84722581833452ecc8943f2641fb5 to 9080506a2f8fa309b175070507953514600ac543
New commits:
9080506  Merge branch 'u/wluebbe/ticket/16409' into 7.3.b6

comment:7 Changed 5 years ago by
 Component changed from build to python3
comment:8 Changed 4 years ago by
 Branch changed from public/16409 to u/chapoton/16409
 Commit changed from 9080506a2f8fa309b175070507953514600ac543 to ba2fdb126734c6c7b603e2d051abc0e96b93bf22
 Milestone changed from sage6.4 to sage8.0
 Status changed from needs_info to needs_review
only the "operator" fixer issue still makes (some) sense.
New commits:
ba2fdb1  py3: silent some 2to3 warnings

comment:9 Changed 4 years ago by
 Cc jdemeyer jhpalmieri added
bot is essentially green. Should be an easy review, I think
comment:10 Changed 4 years ago by
ping ?
comment:11 Changed 4 years ago by
 Reviewers set to André Apitzsch
 Status changed from needs_review to positive_review
LGTM.
comment:12 Changed 4 years ago by
 Branch changed from u/chapoton/16409 to ba2fdb126734c6c7b603e2d051abc0e96b93bf22
 Resolution set to fixed
 Status changed from positive_review to closed
The redefinition of
next()
insage/combinat/integer_list.py
has the textINTERNAL FUNCTION! DO NOT USE DIRECTLY!
. At the top of the module it saysTravis, what would you suggest?  The deprecation has not yet happened, but I do not find it obvious how to fix ...
New commits:
trac #16409: 2to3 refactoring tool warnings  fix "absolute_import"
trac #16409: 2to3 refactoring tool warnings  fix "map"