Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
jhpalmieri opened this issue Apr 17, 2019 · 16 comments

Comments

@jhpalmieri
Copy link
Member

The overarching goal of this ticket is to make this change:

diff --git a/src/sage/doctest/forker.py b/src/sage/doctest/forker.py
index 4b222dd1db..d97034090f 100644
--- a/src/sage/doctest/forker.py
+++ b/src/sage/doctest/forker.py
@@ -2377,20 +2377,11 @@ class DocTestTask(object):
         sage: sorted(results.keys())
         ['cputime', 'err', 'failures', 'optionals', 'tests', 'walltime', 'walltime_skips']
     """
-
-    if six.PY2:
-        extra_globals = {}
-    else:
-        extra_globals = {'long': int}
+    extra_globals = {}
     """
     Extra objects to place in the global namespace in which tests are run.
     Normally this should be empty but there are special cases where it may
     be useful.
-
-    In particular, on Python 3 add ``long`` as an alias for ``int`` so that
-    tests that use the ``long`` built-in (of which there are many) still pass.
-    We do this so that the test suite can run on Python 3 while Python 2 is
-    still the default.
     """
 
     def __init__(self, source):

With Python 3 doctesting, long has been automatically converted to int. It would be better to not have such a big difference in the behavior of doctesting vs. ordinary Sage usage: with Python 3, evaluating long(3) would fail at the command line but work in doctesting.

First we should deprecate __long__ methods for Sage classes, and we should also designate doctests involving long(...) as being py2 only. Once enough of those tasks have been complete, we can make this change to the doctesting framework.


See #27826 for a collection of deprecations. This is not complete: that ticket does not deprecate __long__ for integers and rational numbers.

See #27829 for marking some doctests with # py2.

CC: @mkoeppe

Component: python3

Author: John Palmieri

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

@jhpalmieri jhpalmieri added this to the sage-8.8 milestone Apr 17, 2019
@jhpalmieri
Copy link
Member Author

Branch: u/jhpalmieri/long

@jhpalmieri
Copy link
Member Author

Commit: 1926330

@jhpalmieri
Copy link
Member Author

comment:2

There are a few places where there should be deprecations but there are not: in rational.pyx and integer.pyx, deprecations lead to deprecations warnings all over the place in the Sage library. In complex_double.pyx, for example, the __long__ method just raises an error, and it seemed that deprecating such a method has no effect: the deprecation warning is not printed, just the error is.


New commits:

1926330trac 27696: first steps in deprecating long and __long__

@embray
Copy link
Contributor

embray commented May 13, 2019

comment:3

This was done this way very intentionally in #24559, in large part to avoid such a massive patch like this.

It would be better I think to wait until Python 2 support is dropped completely.

@embray embray removed this from the sage-8.8 milestone May 13, 2019
@jhpalmieri
Copy link
Member Author

comment:4

Right, but that was a year ago. I see no reason to wait until Python 2 support is dropped completely, although if you prefer, we can do this incrementally: say, first add # py2 to a bunch of doctests of the form

sage: long(blah)

We should also deprecate __long__ for various classes. When the appropriate pieces are in place, turn off the conversion long --> int in Python 3 doctesting.

The current situation is very misleading, where a doctest will pass with Python 3 but the same code will fail if run by hand.

@embray
Copy link
Contributor

embray commented May 13, 2019

comment:5

Replying to @jhpalmieri:

The current situation is very misleading, where a doctest will pass with Python 3 but the same code will fail if run by hand.

There are many cases like that where the doctests don't work quite the same on Python 3 but in mostly trivial ways which are made transparent by the doctest framework. I think that's ok, personally.

However, I do like the idea of adding deprecation warnings to __long__. What if as a middle ground we started with just that part?

@jhpalmieri

This comment has been minimized.

@jhpalmieri
Copy link
Member Author

Changed branch from u/jhpalmieri/long to none

@jhpalmieri
Copy link
Member Author

Changed commit from 1926330 to none

@jhpalmieri jhpalmieri changed the title py3: deprecate long and __long__, and remove doctesting special-case for long metaticket: py3: deprecate long and __long__, and remove doctesting special-case for long May 13, 2019
@jhpalmieri

This comment has been minimized.

@jhpalmieri

This comment has been minimized.

@jdemeyer
Copy link

comment:9

Replying to @embray:

However, I do like the idea of adding deprecation warnings to __long__.

I disagree, we shouldn't deprecate __long__, it's a standard Python feature. But let's discuss at #27826.

@jdemeyer
Copy link

comment:10

Also, I don't see how deprecating __long__ helps to achieve the goal of this ticket. Adding the # py2 flags to all doctests involving long(...) should be sufficient.

@jhpalmieri
Copy link
Member Author

comment:11

#27826 may now take care of this entire issue. If #27826 is merged, this can be closed.

@fchapoton
Copy link
Contributor

comment:12

shall we close this one too ?

@jhpalmieri
Copy link
Member Author

comment:13

Yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants