py3: better backwards-compatibility for Python longs

[embray]

This does three little things to better support long-related functionality, especially in the doctests. This allows a lot of tests to pass on Python 3 that wouldn't otherwise, or would need to be skipped and/or have Python 3 specific copies:

• The Sage preparser supports Python 2 style long literals, like 42L. These are just treated as normal ints on Python 3 (we could also consider adding a deprecation warning for this usage). I actually wish Python itself had done this (just as they restored backwards-compat for u'' literals--that they didn't I think is just that explicit longs is relatively uncommon.

• The doctest parser has a fixup for recognizing long literals in the expected output of a test, and treating them as normal ints on Python 3. This is probably the weakest part of the patch in terms of implementation--it's very naïve and I can imagine the possibility of false positives, though it is still pretty unlikely. Open to better ideas on this.

• In sage.all I added, on Python 3 only, a new global called long which is just a function that wraps int() and (except in the tests, for now) raises a DeprecationWarning for using long(). This still allows a lot of tests to work. I deliberately made it a simple function, and not a class, as the latter would tempt too many errors with code like isinstance(x, long).

[git]

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

​e3c14f3

Properly handle cases like '1rL'

[jdemeyer]

I don't really with the condition deprecation depending on DOCTEST_MODE. Doctests should be as close as possible to a normal interactive session. I'm fine with the long = int alias, but then I would remove the deprecation.

[embray]

Replying to jdemeyer:

I don't really with the condition deprecation depending on DOCTEST_MODE. Doctests should be as close as possible to a normal interactive session. I'm fine with the long = int alias, but then I would remove the deprecation.

This was added very deliberately in part to help with porting the doctests, so as to allow the great many doctests that use long() to continue working for now without interruption (while at the same time giving a useful warning for user code).

My intention, though not explicitly stated, was to later (once we want to switch Sage to Python 3) remove the condition so that it's easy to find and fix/remove such tests later.

[jdemeyer]

Replying to embray:

My intention, though not explicitly stated, was to later (once we want to switch Sage to Python 3) remove the condition so that it's easy to find and fix/remove such tests later.

I think it's better to just enable the alias now without any deprecation and then later add an unconditional deprecation. Since Sage doesn't really support Python 3 for the moment, the deprecation won't have much effect anyway.

[embray]

Why do you think that's better? Why is this wrong?

[embray]

There's a difference between "supporting Python 3" and "having Python 3 as the default Sage". I think in some version--hopefully before long (I'm running pretty low on obvious errors to fix, and most of the problems I'm seeing are minor issues in doctests) there will be a Sage that supports Python 3.

For backwards compatibility we'll want to keep Python 2 as the default for some time, but I would definitely encourage users to move to Python 3 sooner rather than later. To that end, I envision that soon Sage will have Python 2 and Python 3 binary releases, with some emphasis on getting people to use the latter. To that end it will be good to help as much as possible with transitioning.

That said, there's also a question of when to switch the default in the docs over. If the docs are Python 3 then I think there should be no long() without a deprecation warning.

In other words, I'd be willing to change it, but it depends a lot on bigger plans regarding how Sage will transition to Python 3 that aren't particularly settled. But I feel right now that this currently with an eye toward being as helpful as possible for transitioning.

[jdemeyer]

Replying to embray:

If the docs are Python 3 then I think there should be no long() without a deprecation warning.

Then it should be the same in doctests too. I'm fine with a deprecation or no deprecation either way, but I am protesting the condition on DOCTEST_MODE.

[embray]

I don't know why. It seems to me the whole point of having DOCTEST_MODE flag is to control behavior in the context of doctests. Obviously this should be used sparingly, but one can always find special cases.

That said, I took a step back from this and rethought it a bit. I thought of adding a long() in sage.all as sort of a helper for transition to Python 3. That said, the lack of long() in Python 3 is hardly the biggest difference, and users aren't often in the habit of using the long built-in very much anyways (especially not in Sage where it's almost entirely unnecessary).

So maybe adding this helper isn't that useful anyways.

So what if we only injected it into the globals when running the doctests, just temporarily, no deprecation warning or anything. Later, when we want to start moving the tests to be more Python 3-compatible, we just remove it and get NameErrors? everywhere the tests try to use it.

[jdemeyer]

OK, I'll stop fighting here. I don't really agree, but if you want to add long on Python 3 if DOCTEST_MODE without deprecation, please go ahead.

[embray]

Thanks--though what do you think about my other proposal of just injecting long for the purpose of the doctests only? I'm personally leaning toward it as a preference.

I understand your feeling that the doctests should reflect what a real user sees. The problem with Sage is that the only tests (with a few exceptions) are the doctests. And for the purposes of porting to Python 3 I don't think that philosophy really applies. It's more important just that the tests are passing so that we can narrow down on real bugs (until we make Python 3 the default, in which case yes we don't want to special case anything in the doctests if possible).

[jdemeyer]

Replying to embray:

Thanks--though what do you think about my other proposal of just injecting long for the purpose of the doctests only? I'm personally leaning toward it as a preference.

Go for it.

[embray]

Why needs work?

Th recent patchbot failures on this ticket are all a joke--they're broken patchbots. This one passed with the current branch on this ticket, and isn't a broken patchbot: ​https://patchbot.sagemath.org/log/24559/Ubuntu/16.04/x86_64/3.13.0-123-generic/sagemath-patchbot-docker/2018-02-04%2014:11:23

[embray]

Ignore me--I did do the proposed work, but it seems I haven't pushed it to the upstream branch yet...

[git]

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

​12ff3be	Fix various minor encoding issues
​bf6cf97	Correct some encoding issues on Python 2.
​4b92b47	A few other miscellaneous fixes to the doctests tests
​ed24910	Various fixes intended to ensure that opened files are closed explictly
​47e464d	Use the restore_atexit context manager for this code to work on Python 3,
​8995cce	On Python 3, allow Python 2-style long literals (they are just treated as normal int literals)
​3a6d2fc	Doctest normalization for Python 2 long literals.
​22c3544	On Python 3 add a 'long' built-in to Sage that acts as a replacement for the Python 2 'long' built-in at least for type conversions.
​45450f3	Properly handle cases like '1rL'
​036a788	py3: instead of providing a user-visible 'long' alias, just inject it into the globals for doctests, just as a transitional measure for the doctests only

[embray]

This depends on #24343, but the new approach to handling long(...) can be see in the latest commit.

Now, instead of providing a user-visible replacement for long (which as I've already remarked is probably not that useful anyways, or at least a slippery-slope to providing other replacements for old Python 2 builtins), it just injects it into the globals for doctests. This should be seen purely as a transitional measure.

---

​036a788

py3: instead of providing a user-visible 'long' alias, just inject it into the globals for doctests, just as a transitional measure for the doctests only

[jdemeyer]

Rebased

---

New commits:

​af86e5c	Various fixes intended to ensure that opened files are closed explictly where
​436ed26	A few other miscellaneous fixes to the doctests tests
​e566901	Use the restore_atexit context manager for this code to work on Python 3,
​6fd2912	Add run option to restore_atexit context
​46017b6	Use restore_atexit(run=True) in doctest framework
​13f433c	make this exception message more flexible to the exact point in the Python interpreter where it occurred
​4a60b3d	Merge commit '436ed266f954ecf03f894b9c6539cca353a68ad6'; commit '13f433cbfd91cd09d2415305b3c54859ef1e983c' into HEAD
​9b24fb6	On Python 3, allow Python 2-style long literals (they are just treated as normal int literals)
​9178517	Doctest normalization for Python 2 long literals.
​8751498	py3: instead of providing a user-visible 'long' alias, just inject it into the globals for doctests, just as a transitional measure for the doctests only

[embray]

Jeroen, I see you rebased this, but does that constitute a "review"?

IIRC, the only open question on this ticket had to do with my plan for aliasing long in Python 3. Originally I was adding an alias in the global namespace for all users (that would raise a deprecation warning). But now I've gone back on that, and am only injecting long as an alias for int into the globals for doctests.

My experience with testing this, so far, has been that it allows a lot of tests to pass on Python 3 that wouldn't otherwise, with few exceptions, and that it's a decent transitional approach.

The extra_globals dict I added could prove useful for a few other things too.

[git]

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

​8cbe1b3	On Python 3, allow Python 2-style long literals (they are just treated as normal int literals)
​5f01333	Doctest normalization for Python 2 long literals.
​e3b8148	py3: instead of providing a user-visible 'long' alias, just inject it into the globals for doctests, just as a transitional measure for the doctests only

[jdemeyer]

I don't like this syntax:

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

It looks too much like a docstring but it isn't. Use a # comment instead.

[embray]

That's standard syntax in Sphinx for documenting class attributes.

[jdemeyer]

Replying to embray:

That's standard syntax in Sphinx for documenting class attributes.

According to the ​Sphinx documentation, Sphinx autodoc also supports the special comment syntax #: for this. And in fact, Sphinx itself uses that syntax.

You may think that there is no harm in using strings-which-are-not-docstrings like that, but the problem is that it teaches a bad habit. In particular the following does not contain a doctest despite looking like it does:

class X(object):
    attr = "something"
    """
    EXAMPLES::
        
        sage: 1 + 1
        3
    """

[embray]

I don't think it's a bad habit. It's perfectly acceptable syntax and I think preferable from the Sphinx perspective over the special comment syntax. If there were a notion in the Python language of attaching a docstring to a particular class attribute that's probably how I'd write it (unfortunately the concept doesn't quite fit in with how the language works, though personally I can think of ways Python might use it).

I think your concern is more a peculiarity of Sage, which overemphasizes doctests in the first place (or, perhaps if this really a problem, the doctest parser should be modified to look in triple-quoted strings anywhere in a class or module body--I think that might make sense anyways).