Opened 5 months ago

Closed 5 months ago

#27585 closed enhancement (fixed)

py3: fixes in matrices

Reported by: jhpalmieri Owned by:
Priority: major Milestone: sage-8.8
Component: python3 Keywords:
Cc: jdemeyer, chapoton, tscrim Merged in:
Authors: John Palmieri Reviewers: Vincent Klein
Report Upstream: N/A Work issues:
Branch: 888612c (Commits) Commit: 888612cf517cdb99c075ba3bdde0dfbfbff75942
Dependencies: Stopgaps:

Description

This fixes a few doctests for sage.matrices:

  • there is some randomness in the ordering in one small part of one verbose test in compute_J_ideal.py, so replace it with ...
  • up to now, mat[range(3), range(4)] did not work, so I've tried to modify __getitem__ and __setitem__ to allow range types as inputs
  • some doctests use random matrices to test other features. The randomness differs on Python 2 vs. Python 3, so get rid of the randomness. The tests still test the relevant methods (which are determinants, for example, not the randomness).
  • there is minor numerical noise in one doctest.

Change History (6)

comment:1 Changed 5 months ago by jhpalmieri

  • Branch set to u/jhpalmieri/matrix-py3

comment:2 Changed 5 months ago by jhpalmieri

  • Cc jdemeyer chapoton tscrim added
  • Commit set to 888612cf517cdb99c075ba3bdde0dfbfbff75942
  • Status changed from new to needs_review

Regarding the random matrix issue: I think what's going on is that the different versions of Python are using different orderings on monomials in QQ.<x,y>, for instance, so after coming up with a list of random coefficients, the resulting polynomials are different. (With Python 2, you would see a*y^2 + b*x*y while in Python 3 you would see a*x*y + b*y^2, same coefficients a and b in the two cases.)

Regarding the changes to allow range(...) as valid input to __getitem__ and __setitem__ for matrices: are these changes okay? Is there a better way to do it?


New commits:

888612ctrac 27585: work on Python 3 compatibility for sage.matrix

comment:3 Changed 5 months ago by jhpalmieri

After this ticket, there is one remaining doctest failure in sage.matrix:

TypeError: type sage.rings.real_double.RealDoubleElement doesn't define __round__ method

in matrix_double_dense.pyx.

comment:4 Changed 5 months ago by vklein

  • Status changed from needs_review to positive_review

patchbot errors are unrelated with this ticket.
Looks good to me.

comment:5 Changed 5 months ago by vklein

  • Reviewers set to Vincent Klein

comment:6 Changed 5 months ago by vbraun

  • Branch changed from u/jhpalmieri/matrix-py3 to 888612cf517cdb99c075ba3bdde0dfbfbff75942
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.