Opened 6 months ago
Last modified 2 months ago
#27826 needs_review enhancement
Deprecate 'long(...)'
Reported by:  jhpalmieri  Owned by:  

Priority:  major  Milestone:  sage8.9 
Component:  python3  Keywords:  
Cc:  embray  Merged in:  
Authors:  John Palmieri  Reviewers:  
Report Upstream:  N/A  Work issues:  Rebase 
Branch:  u/jhpalmieri/deprecatelong (Commits)  Commit:  ce2eb939124f015e888a1df6a2823318341e1b7e 
Dependencies:  Stopgaps: 
Description (last modified by )
This is part of #27696. long(...)
is not defined in Python 3, and so long
should be deprecated in Sage built with Python 2.
Change History (34)
comment:1 Changed 6 months ago by
 Branch set to u/jhpalmieri/deprecatelong
comment:2 Changed 6 months ago by
 Commit set to d5fcb32c0b0100383e09754d319eb28ba18c5862
 Status changed from new to needs_review
comment:3 in reply to: ↑ description ; followup: ↓ 5 Changed 6 months ago by
 Status changed from needs_review to needs_info
Replying to jhpalmieri:
Notable classes which are not included: integers and rational numbers. This is mainly because those classes are used in so many places in the Sage library that deprecation warnings for them would get triggered many times during doctesting.
Are there really that many places where an integer/rational is explicitly converted to a long
? That worries me. I really want to understand better why you're not applying this everywhere (needs_info for that reason).
About the patch itself:
 in
src/sage/libs/mpmath/ext_main.pyx
you messed up the spacing of"""
 I don't understand the comment
This is deprecated, but the ``TypeError`` takes precedence over the deprecation warning when this is run.
How does the error "take precedence" over the warning, what are you trying to say?
comment:4 followup: ↓ 6 Changed 6 months ago by
Regarding integer/rational: if I add deprecation warnings to those, I get over 800 files with failures. Many involve set_random_seed
or other uses of randomization:
sage t long src/sage/parallel/reference.py ********************************************************************** File "src/sage/parallel/reference.py", line 34, in sage.parallel.reference.parallel_iter Failed example: set_random_seed(0) Expected nothing Got: doctest:warning File "/Users/jpalmier/Desktop/Sage/git/sage/src/bin/sageruntests", line 179, in <module> err = DC.run() File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/sitepackages/sage/doctest/control.py", line 1232, in run self.run_doctests() File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/sitepackages/sage/doctest/control.py", line 933, in run_doctests self.dispatcher.dispatch() File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 2000, in dispatch self.parallel_dispatch() File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 1897, in parallel_dispatch w.start() # This might take some time File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 2183, in start super(DocTestWorker, self).start() File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/multiprocessing/process.py", line 130, in start self._popen = Popen(self) File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/multiprocessing/forking.py", line 126, in __init__ code = process_obj._bootstrap() File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/multiprocessing/process.py", line 267, in _bootstrap self.run() File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 2139, in run task(self.options, self.outtmpfile, msgpipe, self.result_queue) File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 2477, in __call__ doctests, extras = self._run(runner, options, results) File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 2526, in _run result = runner.run(test) File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 869, in run return self._run(test, compileflags, out) File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 671, in _run self.compile_and_execute(example, compiler, test.globs) File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 1095, in compile_and_execute exec(compiled, globs) File "<doctest sage.parallel.reference.parallel_iter[2]>", line 1, in <module> set_random_seed(Integer(0)) File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/sitepackages/sage/misc/superseded.py", line 102, in deprecation warning(trac_number, message, DeprecationWarning, stacklevel) File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/sitepackages/sage/misc/superseded.py", line 148, in warning warn(message, warning_class, stacklevel) : DeprecationWarning: use of long is deprecated, since long() will no longer be supported in Python 3 See https://trac.sagemath.org/27826 for details. **********************************************************************
An example without randomness:
sage t long src/sage/quadratic_forms/extras.py ********************************************************************** File "src/sage/quadratic_forms/extras.py", line 50, in sage.quadratic_forms.extras.is_triangular_number Failed example: F1 = [n for n in range(1,100*(100+1)/2) if is_triangular_number(n)] Expected nothing Got: doctest:warning File "/Users/jpalmier/Desktop/Sage/git/sage/src/bin/sageruntests", line 179, in <module> err = DC.run() File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/sitepackages/sage/doctest/control.py", line 1232, in run self.run_doctests() File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/sitepackages/sage/doctest/control.py", line 933, in run_doctests self.dispatcher.dispatch() File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 2000, in dispatch self.parallel_dispatch() File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 1897, in parallel_dispatch w.start() # This might take some time File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 2183, in start super(DocTestWorker, self).start() File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/multiprocessing/process.py", line 130, in start self._popen = Popen(self) File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/multiprocessing/forking.py", line 126, in __init__ code = process_obj._bootstrap() File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/multiprocessing/process.py", line 267, in _bootstrap self.run() File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 2139, in run task(self.options, self.outtmpfile, msgpipe, self.result_queue) File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 2477, in __call__ doctests, extras = self._run(runner, options, results) File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 2526, in _run result = runner.run(test) File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 869, in run return self._run(test, compileflags, out) File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 671, in _run self.compile_and_execute(example, compiler, test.globs) File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 1095, in compile_and_execute exec(compiled, globs) File "<doctest sage.quadratic_forms.extras.is_triangular_number[7]>", line 1, in <module> F1 = [n for n in range(Integer(1),Integer(100)*(Integer(100)+Integer(1))/Integer(2)) File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/sitepackages/sage/misc/superseded.py", line 102, in deprecation warning(trac_number, message, DeprecationWarning, stacklevel) File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/sitepackages/sage/misc/superseded.py", line 148, in warning warn(message, warning_class, stacklevel) : DeprecationWarning: use of long is deprecated, since long() will no longer be supported in Python 3 See https://trac.sagemath.org/27826 for details.
comment:5 in reply to: ↑ 3 ; followup: ↓ 7 Changed 6 months ago by
Replying to jdemeyer:
About the patch itself:
 in
src/sage/libs/mpmath/ext_main.pyx
you messed up the spacing of"""
Easy to fix.
 I don't understand the comment
This is deprecated, but the ``TypeError`` takes precedence over the deprecation warning when this is run.
How does the error "take precedence" over the warning, what are you trying to say?
By "take precedence", I mean that no deprecation warning is printed, just the TypeError
. I'm trying to explain why there is no doctest with a deprecation warning. That could be clearer.
comment:6 in reply to: ↑ 4 ; followup: ↓ 9 Changed 6 months ago by
Replying to jhpalmieri:
Regarding integer/rational: if I add deprecation warnings to those, I get over 800 files with failures. Many involve
set_random_seed
or other uses of randomization
The question is why do you get so many failures?
comment:7 in reply to: ↑ 5 Changed 6 months ago by
Replying to jhpalmieri:
By "take precedence", I mean that no deprecation warning is printed, just the
TypeError
. I'm trying to explain why there is no doctest with a deprecation warning. That could be clearer.
I would just remove that note, it's fine.
comment:8 Changed 6 months ago by
 Commit changed from d5fcb32c0b0100383e09754d319eb28ba18c5862 to 2a1b5e40057252c426a7716408e89d1e097540fa
Branch pushed to git repo; I updated commit sha1. New commits:
2a1b5e4  trac 27826: fix typo

comment:9 in reply to: ↑ 6 ; followup: ↓ 11 Changed 6 months ago by
Replying to jdemeyer:
Replying to jhpalmieri:
Regarding integer/rational: if I add deprecation warnings to those, I get over 800 files with failures. Many involve
set_random_seed
or other uses of randomizationThe question is why do you get so many failures?
I would prefer to address that on another ticket: deal with the easy cases here (and also #27829) and then think about ZZ and QQ.
I haven't investigated at all, just saw that it was a mess and so I wanted to put it off. Does Python's range
try to convert to long
in Python 2? Or do some other Python builtins do that?
comment:10 Changed 6 months ago by
 Commit changed from 2a1b5e40057252c426a7716408e89d1e097540fa to c70dce2a8effb5723cf1d6ab4cd5f798986f3fb4
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
c70dce2  trac 27826: fix typo, remove some unneeded NOTEs.

comment:11 in reply to: ↑ 9 Changed 6 months ago by
 Status changed from needs_info to needs_work
Replying to jhpalmieri:
I would prefer to address that on another ticket: deal with the easy cases here (and also #27829) and then think about ZZ and QQ.
1
We clearly have no clue what we're doing or what the consequences are of that deprecation.
comment:12 Changed 6 months ago by
I have no idea what your objection is or what work you think needs doing on this ticket. Can you explain?
comment:13 Changed 6 months ago by
In some cases (for integers and rationals), deprecating __long__
causes a large number of warnings. We don't know why there are so many warnings. So deprecating __long__
seems to have unforeseen consequences. This is a sign that we should be prudent when deprecating __long__
.
I would much prefer to deal with all cases of __long__
together (this does not necessarily mean on one ticket, but it does mean understanding the consequences of deprecating all __long__
methods).
We might also make a difference between plain Python classes and Cython cdef classes.
comment:14 followup: ↓ 16 Changed 6 months ago by
What's your plan for external packages calling long(...)
? For example, fpylll
does this and I don't think it's wrong for them to do that.
This is especially true from the point of view of Cython and the Python/C API, where the long
class in Python 2 is the same as the int
class in Python 3 (you might think that Python 3 removed the long
class but what really happened is they removed int
and then renamed long
> int
).
So I'm now leaning towards closing this ticket as "wontfix".
comment:15 Changed 6 months ago by
Also numpy uses long
internally (in their C code at least).
comment:16 in reply to: ↑ 14 ; followup: ↓ 18 Changed 6 months ago by
As far as I can tell, __long__
methods are not used in Python 3 (despite what you say about the renaming long > int
, int(x)
does not call x.__long__()
). Let me add a new goal to this, or rather make explicit a previously implicit goal: we want users to stop using the long()
function, in order to ease the transition to Python 3. Deprecating __long__
is a way to do this. Is there a better way to accomplish this?
comment:17 followup: ↓ 19 Changed 6 months ago by
I would point out that there are many calls to long
in sage.misc.randstate
, so fixing those will help with some of these issues.
comment:18 in reply to: ↑ 16 ; followup: ↓ 21 Changed 6 months ago by
Could we replace long()
in the REPL by a custom long()
which shows a deprecation warning and then calls the real long()
?
comment:19 in reply to: ↑ 17 Changed 6 months ago by
Replying to jhpalmieri:
I would point out that there are many calls to
long
insage.misc.randstate
, so fixing those will help with some of these issues.
But I don't agree that those calls are problematic. We really need a long
there (or int
in Python 3 terminology), so calling long()
(which Cython understands as int
on Python 3) seems the right thing to do.
comment:20 Changed 6 months ago by
 Commit changed from c70dce2a8effb5723cf1d6ab4cd5f798986f3fb4 to 9eafb320af9eabcbea251dc6ed1f4e042e8f3717
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
9eafb32  trac 27826: in Python 2, deprecate the "long" function.

comment:21 in reply to: ↑ 18 Changed 6 months ago by
Replying to jdemeyer:
Could we replace
long()
in the REPL by a customlong()
which shows a deprecation warning and then calls the reallong()
?
Here's a version which attempts this.
comment:22 Changed 6 months ago by
 Status changed from needs_work to needs_review
comment:23 Changed 6 months ago by
 Description modified (diff)
 Summary changed from Deprecate __long__ for many classes to Deprecate 'long(...)'
comment:24 Changed 6 months ago by
 Description modified (diff)
comment:26 Changed 6 months ago by
Has this been tested on Python 3? I fear it might have problems on Python 3, ironically, due to #24755 but I'm not sure.
comment:27 Changed 6 months ago by
Most of the new deprecation warnings are doctested with tests tagged # py2
, so this shouldn't cause any issues with #24755. I believe as I was developing this, I was testing in parallel with both Python 2 and 3. I'm doublechecking now...
comment:28 Changed 6 months ago by
I've just run doctests with Python 3, and I don't see any new failures coming from this ticket. make ptestpython3
passes, and running tests on the other files changed here doesn't reveal any new problems.
comment:29 Changed 5 months ago by
Ping
comment:30 Changed 5 months ago by
 Milestone changed from sage8.8 to sage8.9
Moving tickets from the Sage 8.8 milestone that have been actively worked on in the last six months to the next release milestone (optimistically).
comment:31 Changed 4 months ago by
 Status changed from needs_review to needs_work
 Work issues set to Rebase
The branch apparently doesn't merge.
comment:32 Changed 4 months ago by
 Commit changed from 9eafb320af9eabcbea251dc6ed1f4e042e8f3717 to ce2eb939124f015e888a1df6a2823318341e1b7e
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
ce2eb93  trac 27826: in Python 2, deprecate the "long" function.

comment:34 Changed 2 months ago by
Any comments from anyone? I just tested with 8.9.rc0 on OS X (after doing git rebase
): no failures with Python 2 or Python 3.
New commits:
trac 27826: deprecate __long__ methods for many Sage classes