#27675 closed enhancement (fixed)

py3 fixes in rings/polynomial/polynomial_element.pyx

Reported by: jhpalmieri Owned by:
Priority: major Milestone: sage-8.8
Component: python3 Keywords:
Cc: Merged in:
Authors: John Palmieri Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 44d4301 (Commits) Commit: 44d4301714d0813a205df645beb4df060188c3cc
Dependencies: Stopgaps:

Description

Mark some long(...) tests as # py2, and also deprecate the __long__ method.

Change History (9)

comment:1 Changed 11 months ago by jhpalmieri

  • Branch set to u/jhpalmieri/poly_element

comment:2 Changed 11 months ago by jhpalmieri

  • Commit set to 70acaf6957a79f53a802c3ad219a2479291a8570
  • Status changed from new to needs_review

New commits:

70acaf6trac 27675: py3 fixes in rings/polynomial/polynomial_element.pyx

comment:3 Changed 11 months ago by chapoton

  • Status changed from needs_review to needs_work

failing doctests, see patchbot

comment:4 Changed 11 months ago by git

  • Commit changed from 70acaf6957a79f53a802c3ad219a2479291a8570 to 44d4301714d0813a205df645beb4df060188c3cc

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

44d4301trac 27675: py3 fixes in rings/polynomial/polynomial_element.pyx

comment:5 Changed 11 months ago by jhpalmieri

This should fix the doctests. A question about Python 3, though: in an interactive session, I see this:

sage: R.<x> = PolynomialRing(QQ)
sage: S.<a> = R.quotient(x^3-2)
sage: long(S(10))
---------------------------------------------------------------------------
NameError                                 Traceback (most recent call last)
<ipython-input-4-be7f7f725292> in <module>()
----> 1 long(S(Integer(10)))

NameError: name 'long' is not defined

But this (from polynomial_quotient_ring_element.py) passes doctests with Python 3:

            sage: R.<x> = PolynomialRing(QQ)
            sage: S.<a> = R.quotient(x^3-2)
            sage: long(S(10))
            10L

In this ticket I've decided to mark that test as # py2, but it doesn't seem to be necessary, and I don't understand why. Any explanations? (I've confirmed that the test is actually being run.)

comment:6 Changed 11 months ago by jhpalmieri

  • Status changed from needs_work to needs_review

comment:7 Changed 11 months ago by jhpalmieri

Here's the answer to my question: in doctest/forker.py, we have

    if six.PY2:
        extra_globals = {}
    else:
        extra_globals = {'long': int}
    """
    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.
    """

I think we should clean this up: Sage's actual behavior should not differ that much from doctest behavior. (Not on this ticket, obviously.)

comment:8 Changed 11 months ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

LGTM.

comment:9 Changed 11 months ago by vbraun

  • Branch changed from u/jhpalmieri/poly_element to 44d4301714d0813a205df645beb4df060188c3cc
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.