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

py3 again some care for range #22816

Closed
fchapoton opened this issue Apr 15, 2017 · 10 comments
Closed

py3 again some care for range #22816

fchapoton opened this issue Apr 15, 2017 · 10 comments

Comments

@fchapoton
Copy link
Contributor

part of #16081

CC: @jdemeyer @tscrim

Component: python3

Author: Frédéric Chapoton

Branch/Commit: be9e797

Reviewer: Travis Scrimshaw

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

@fchapoton fchapoton added this to the sage-8.0 milestone Apr 15, 2017
@fchapoton
Copy link
Contributor Author

New commits:

be9e797some more care for range..

@fchapoton
Copy link
Contributor Author

Branch: u/chapoton/22816

@fchapoton
Copy link
Contributor Author

Commit: be9e797

@tscrim
Copy link
Collaborator

tscrim commented Apr 16, 2017

comment:2

The only change I am not sure about is this one:

diff --git a/src/sage/matrix/matrix0.pyx b/src/sage/matrix/matrix0.pyx
index c5959f0..50b5d00 100644
--- a/src/sage/matrix/matrix0.pyx
+++ b/src/sage/matrix/matrix0.pyx
@@ -5177,7 +5177,7 @@ cdef class Matrix(sage.structure.element.Matrix):
             if not B[self._nrows-1, self._ncols-1]:
                 raise ZeroDivisionError("input matrix must be nonsingular")
 
-        return B.matrix_from_columns(range(self._ncols, 2*self._ncols))
+        return B.matrix_from_columns(list(xrange(self._ncols, 2 * self._ncols)))
 
     def __pos__(self):
         """

I forget if Cython keeps the behavior that range will return a list in Python3. If it will, then I'm good with the change, otherwise, then I'd like to know if Cython optimizes this.

@tscrim
Copy link
Collaborator

tscrim commented Apr 17, 2017

comment:3

ping?

@fchapoton
Copy link
Contributor Author

comment:4

I have no idea..

@fchapoton
Copy link
Contributor Author

comment:5

There are already a few list(xrange(...)) in the very same file..

@tscrim
Copy link
Collaborator

tscrim commented Apr 17, 2017

comment:6

Fair point.

@tscrim
Copy link
Collaborator

tscrim commented Apr 17, 2017

Reviewer: Travis Scrimshaw

@vbraun
Copy link
Member

vbraun commented Apr 23, 2017

Changed branch from u/chapoton/22816 to be9e797

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

3 participants