Remove calls to 'long(...)', etc. #27826

jhpalmieri · 2019-05-13T20:08:27Z

long(...) is not defined in Python 3, and so we should remove all calls to long. We should also delete all methods __long__, and remove the special-casing for long in Python 3 doctesting (in doctest/forker.py).

Sage 9.2 will not support Python 2 anymore, so that is this ticket's target release.

CC: @embray @fchapoton

Component: python3

Work Issues: Rebase

Author: John Palmieri

Branch/Commit: 3fcba90

Reviewer: Matthias Koeppe

Issue created by migration from https://trac.sagemath.org/ticket/27826

The text was updated successfully, but these errors were encountered:

jhpalmieri · 2019-05-13T21:29:57Z

Branch: u/jhpalmieri/deprecate-long

jhpalmieri · 2019-05-13T22:41:14Z

Commit: d5fcb32

jhpalmieri · 2019-05-13T22:41:14Z

New commits:

`d5fcb32`	trac 27826: deprecate `__long__` methods for many Sage classes

jdemeyer · 2019-05-15T08:17:58Z

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?

jhpalmieri · 2019-05-15T18:09:23Z

comment:4

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/sage-runtests", line 179, in <module>
        err = DC.run()
      File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/site-packages/sage/doctest/control.py", line 1232, in run
        self.run_doctests()
      File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/site-packages/sage/doctest/control.py", line 933, in run_doctests
        self.dispatcher.dispatch()
      File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 2000, in dispatch
        self.parallel_dispatch()
      File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/site-packages/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/site-packages/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/site-packages/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/site-packages/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/site-packages/sage/doctest/forker.py", line 2526, in _run
        result = runner.run(test)
      File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/site-packages/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/site-packages/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/site-packages/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/site-packages/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/site-packages/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/sage-runtests", line 179, in <module>
        err = DC.run()
      File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/site-packages/sage/doctest/control.py", line 1232, in run
        self.run_doctests()
      File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/site-packages/sage/doctest/control.py", line 933, in run_doctests
        self.dispatcher.dispatch()
      File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 2000, in dispatch
        self.parallel_dispatch()
      File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/site-packages/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/site-packages/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/site-packages/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/site-packages/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/site-packages/sage/doctest/forker.py", line 2526, in _run
        result = runner.run(test)
      File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/site-packages/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/site-packages/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/site-packages/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/site-packages/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/site-packages/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.

jhpalmieri · 2019-05-15T18:10:15Z

comment:5

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.

jdemeyer · 2019-05-15T18:21:39Z

comment:6

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?

jdemeyer · 2019-05-15T18:45:27Z

comment:7

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.

sagetrac-git · 2019-05-15T20:04:46Z

Branch pushed to git repo; I updated commit sha1. New commits:

`2a1b5e4`	`trac 27826: fix typo`

sagetrac-git · 2019-05-15T20:04:46Z

Changed commit from d5fcb32 to 2a1b5e4

jhpalmieri · 2019-05-15T20:08:07Z

comment:9

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 randomization

The 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 built-ins do that?

sagetrac-git · 2019-05-15T20:08:56Z

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.`

sagetrac-git · 2019-05-15T20:08:56Z

Changed commit from 2a1b5e4 to c70dce2

jdemeyer · 2019-05-16T05:45:02Z

comment:11

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.

jhpalmieri · 2019-05-16T06:35:27Z

comment:12

I have no idea what your objection is or what work you think needs doing on this ticket. Can you explain?

jdemeyer · 2019-05-16T08:37:43Z

comment:13

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.

jdemeyer · 2019-05-16T12:13:36Z

comment:14

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".

jdemeyer · 2019-05-16T12:57:39Z

comment:15

Also numpy uses long internally (in their C code at least).

jhpalmieri · 2019-05-16T15:14:18Z

comment:16

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?

jhpalmieri · 2019-05-16T15:25:58Z

comment:17

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.

jdemeyer · 2019-05-16T15:28:35Z

comment:18

Could we replace long() in the REPL by a custom long() which shows a deprecation warning and then calls the real long()?

sagetrac-git · 2020-04-19T22:47:24Z

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

`8b76372`	trac 27826: remove `__long__` methods, calls to long in doctests,

sagetrac-git · 2020-04-19T22:47:24Z

Changed commit from ce2eb93 to 8b76372

sagetrac-git · 2020-04-19T22:52:57Z

Changed commit from 8b76372 to fc4c0fa

sagetrac-git · 2020-04-19T22:52:57Z

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

`fc4c0fa`	trac 27826: remove `__long__` methods, calls to long in doctests,

jhpalmieri · 2020-04-19T22:57:17Z

comment:40

If this is merged, we can close #27696 and #27829.

mkoeppe · 2020-06-14T20:31:26Z

comment:41

Needs rebasing

sagetrac-git · 2020-06-14T22:10:10Z

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

`3fcba90`	trac 27826: remove `__long__` methods, calls to long in doctests,

sagetrac-git · 2020-06-14T22:10:10Z

Changed commit from fc4c0fa to 3fcba90

jhpalmieri · 2020-06-14T22:10:58Z

comment:43

Rebased.

mkoeppe · 2020-06-14T22:39:38Z

comment:44

Looks good to me but it would be good if our py3 transition experts could take a look too

mkoeppe · 2020-06-18T16:10:11Z

Reviewer: Matthias Koeppe

jhpalmieri · 2020-06-18T17:20:32Z

comment:46

Thank you! This one has been waiting for a while.

vbraun · 2020-06-22T22:34:27Z

Changed branch from u/jhpalmieri/deprecate-long to 3fcba90

jhpalmieri added this to the sage-8.8 milestone May 13, 2019

jhpalmieri added c: python3 labels May 13, 2019

jhpalmieri added the s: needs review label May 13, 2019

jdemeyer added s: needs info and removed s: needs review labels May 15, 2019

jdemeyer added s: needs work and removed s: needs info labels May 16, 2019

embray modified the milestones: sage-8.9, sage-9.1 Dec 30, 2019

mkoeppe modified the milestones: sage-9.1, sage-9.2 Apr 15, 2020

This comment has been minimized.

Sign in to view

jhpalmieri changed the title ~~Deprecate 'long(...)'~~ Remove calls to 'long(...)', etc. Apr 19, 2020

mkoeppe added s: needs work and removed s: needs review labels Jun 14, 2020

jhpalmieri added s: needs review and removed s: needs work labels Jun 14, 2020

mkoeppe added s: positive review and removed s: needs review labels Jun 18, 2020

vbraun removed the s: positive review label Jun 22, 2020

vbraun closed this as completed in f8dd963 Jun 22, 2020

This was referenced Aug 19, 2020

metaticket: py3: deprecate long and __long__, and remove doctesting special-case for long #27696

Closed

Mark some tests involving long using the # py2 tag #27829

Closed

mkoeppe mentioned this issue Aug 22, 2020

Meta-ticket: Remove Python 2 support for Sage 9.2; upgrades enabled by the removal #29141

Closed

Remove calls to 'long(...)', etc. #27826

Remove calls to 'long(...)', etc. #27826

Comments

jhpalmieri commented May 13, 2019

jhpalmieri commented May 13, 2019

jhpalmieri commented May 13, 2019

jhpalmieri commented May 13, 2019

jdemeyer commented May 15, 2019

jhpalmieri commented May 15, 2019

jhpalmieri commented May 15, 2019

jdemeyer commented May 15, 2019

jdemeyer commented May 15, 2019

sagetrac-git mannequin commented May 15, 2019

sagetrac-git mannequin commented May 15, 2019

jhpalmieri commented May 15, 2019

sagetrac-git mannequin commented May 15, 2019

sagetrac-git mannequin commented May 15, 2019

jdemeyer commented May 16, 2019

jhpalmieri commented May 16, 2019

jdemeyer commented May 16, 2019

jdemeyer commented May 16, 2019

jdemeyer commented May 16, 2019

jhpalmieri commented May 16, 2019

jhpalmieri commented May 16, 2019

jdemeyer commented May 16, 2019

sagetrac-git mannequin commented Apr 19, 2020

sagetrac-git mannequin commented Apr 19, 2020

This comment has been minimized.

sagetrac-git mannequin commented Apr 19, 2020

sagetrac-git mannequin commented Apr 19, 2020

jhpalmieri commented Apr 19, 2020

mkoeppe commented Jun 14, 2020

sagetrac-git mannequin commented Jun 14, 2020

sagetrac-git mannequin commented Jun 14, 2020

jhpalmieri commented Jun 14, 2020

mkoeppe commented Jun 14, 2020

mkoeppe commented Jun 18, 2020

jhpalmieri commented Jun 18, 2020

vbraun commented Jun 22, 2020