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

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

Closed
jhpalmieri opened this issue May 13, 2019 · 19 comments
Closed

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

jhpalmieri opened this issue May 13, 2019 · 19 comments

Comments

@jhpalmieri
Copy link
Member

Since long is not available in Python 3, we should mark tests involving it as for Python 2 only. This is part of #27696.

Depends on #27826

Component: python3

Author: John Palmieri

Branch/Commit: u/jhpalmieri/long-py2 @ feac66d

Reviewer: Matthias Koeppe

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

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

Branch: u/jhpalmieri/long-py2

@jhpalmieri
Copy link
Member Author

Commit: 422e759

@jhpalmieri
Copy link
Member Author

New commits:

422e759trac 27829: add "# py2" to some tests involving "long(...)"

@jhpalmieri
Copy link
Member Author

comment:3

There are a few cases I didn't know what to do with. In the following, I don't think long(...) adds much, so I think it's okay to make these changes. I have not yet done so. Opinions?

diff --git a/src/sage/rings/complex_double.pyx b/src/sage/rings/complex_double.pyx
index ec43d0e199..21c9818780 100644
--- a/src/sage/rings/complex_double.pyx
+++ b/src/sage/rings/complex_double.pyx
@@ -398,7 +398,7 @@ cdef class ComplexDoubleField_class(sage.rings.ring.Field):
 
             sage: CDF(1) + RR(1)
             2.0
-            sage: CDF.0 - CC(1) - long(1) - RR(1) - QQbar(1)
+            sage: CDF.0 - CC(1) - ZZ(1) - RR(1) - QQbar(1)
             -4.0 + 1.0*I
             sage: CDF.has_coerce_map_from(ComplexField(20))
             False
diff --git a/src/sage/modules/free_module_element.pyx b/src/sage/modules/free_module_element.pyx
index 03d4d57a28..e069377806 100644
--- a/src/sage/modules/free_module_element.pyx
+++ b/src/sage/modules/free_module_element.pyx
@@ -382,7 +382,7 @@ def vector(arg0, arg1=None, arg2=None, sparse=None):
         (0, 1, 2, 3, 4, 5, 6, 7, 8, 9)
         sage: v[3].parent()
         Integer Ring
-        sage: v = vector([float(23.4), int(2), complex(2+7*I), long(1)]); v
+        sage: v = vector([float(23.4), int(2), complex(2+7*I), 1]); v
         (23.4, 2.0, 2.0 + 7.0*I, 1.0)
         sage: v[1].parent()
         Complex Double Field
diff --git a/src/sage/rings/real_mpfr.pyx b/src/sage/rings/real_mpfr.pyx
index ae74119795..a579b5a575 100644
--- a/src/sage/rings/real_mpfr.pyx
+++ b/src/sage/rings/real_mpfr.pyx
@@ -719,7 +719,7 @@ cdef class RealField_class(sage.rings.ring.Field):
 
         TESTS::
 
-            sage: 1.0 - ZZ(1) - int(1) - long(1) - QQ(1) - RealField(100)(1) - AA(1) - RLF(1)
+            sage: 1.0 - ZZ(1) - int(1) - 1 - QQ(1) - RealField(100)(1) - AA(1) - RLF(1)
             -6.00000000000000
             sage: R = RR['x']   # Hold reference to avoid garbage collection, see Trac #24709
             sage: R.get_action(ZZ)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 15, 2019

Changed commit from 422e759 to feac66d

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 15, 2019

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

feac66dtrac 27829: add "# py2" to some tests involving "long(...)"

@jdemeyer
Copy link

comment:5

I don't think that # optional - fricas py2 works the way you think it does: you need # py2 not # optional - py2.

@jdemeyer
Copy link

comment:6

For large numbers (which don't fit in a Python 2 int anyway), you can replace long() by int(), for example

a = long(-901824309821093821093812093810928309183091832091)

becomes

a = int(-901824309821093821093812093810928309183091832091)

(unless the goal of the doctest is precisely testing __long__ which is not the case here).

@jhpalmieri
Copy link
Member Author

comment:7

Replying to @jdemeyer:

I don't think that # optional - fricas py2 works the way you think it does: you need # py2 not # optional - py2.

It works exactly the way I think it does, I just made a mistake.

@jhpalmieri
Copy link
Member Author

comment:8

Any changes in #27826 are likely to affect this ticket, so marking as "needs work" until #27826 has stabilized.

@embray
Copy link
Contributor

embray commented Jun 14, 2019

comment:9

Tickets still needing working or clarification should be moved to the next release milestone at the soonest (please feel free to revert if you think the ticket is close to being resolved).

@embray embray modified the milestones: sage-8.8, sage-8.9 Jun 14, 2019
@vbraun
Copy link
Member

vbraun commented Nov 30, 2019

comment:10

Surely these all have been fixed by now and this ticket can be closed?

@jhpalmieri
Copy link
Member Author

comment:11

No, because we still have

    if six.PY2:
        extra_globals = {}
    else:
        extra_globals = {'long': int}

in doctest/forker.py, so tests involving long can still pass with Python 3.

#27826 should be resolved first, and then this ticket afterwards.

@jhpalmieri
Copy link
Member Author

Dependencies: #27826

@embray
Copy link
Contributor

embray commented Dec 30, 2019

comment:12

Ticket retargeted after milestone closed

@embray embray modified the milestones: sage-8.9, sage-9.1 Dec 30, 2019
@mkoeppe
Copy link
Contributor

mkoeppe commented May 1, 2020

comment:13

Moving tickets to milestone sage-9.2 based on a review of last modification date, branch status, and severity.

@mkoeppe mkoeppe modified the milestones: sage-9.1, sage-9.2 May 1, 2020
@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 19, 2020

comment:14

Replying to @jhpalmieri:

#27826 should be resolved first, and then this ticket afterwards.

This has happened

@jhpalmieri
Copy link
Member Author

comment:15

I think we can close this as invalid now that we don't build Python 2 anymore.

@jhpalmieri jhpalmieri removed this from the sage-9.2 milestone Jul 19, 2020
@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 20, 2020

Reviewer: Matthias Koeppe

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

6 participants