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

method to reverse row and column orders of a matrix #23050

Closed
videlec opened this issue May 22, 2017 · 27 comments
Closed

method to reverse row and column orders of a matrix #23050

videlec opened this issue May 22, 2017 · 27 comments

Comments

@videlec
Copy link
Contributor

videlec commented May 22, 2017

This ticket implements a method reverse_rows_and_columns

sage: m = matrix(ZZ, 2, 3, range(6))
sage: m.reverse_rows_and_columns()
sage: m
[5 4 3]
[2 1 0]

Needed in #22841.

Note: in numpy they rather use flip (that return a copy) and Python list uses reverse

CC: @kwankyu

Component: linear algebra

Author: Vincent Delecroix

Branch/Commit: e7d56d0

Reviewer: Kwankyu Lee

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

@videlec videlec added this to the sage-8.0 milestone May 22, 2017
@videlec
Copy link
Contributor Author

videlec commented May 22, 2017

Commit: 17bbf4e

@videlec
Copy link
Contributor Author

videlec commented May 22, 2017

Branch: u/vdelecroix/23050

@videlec
Copy link
Contributor Author

videlec commented May 22, 2017

New commits:

17bbf4e23050: reverse_rows_and_columns

@videlec

This comment has been minimized.

@mforets
Copy link
Mannequin

mforets mannequin commented May 22, 2017

comment:2

salut Vincent, i saw your cool function, maybe you can paste this too or similar:

The row-and-column-reversed matrix of `M` is defined as:

.. MATH::

    (i,j) \mapsto M[nrows - i - 1, ncols - j - 1]

In other words, this function flips `M` up to down and left to right.

@videlec
Copy link
Contributor Author

videlec commented May 22, 2017

comment:3

Replying to @mforets:

salut Vincent, i saw your cool function, maybe you can paste this too or similar:

The row-and-column-reversed matrix of `M` is defined as:

.. MATH::

    (i,j) \mapsto M[nrows - i - 1, ncols - j - 1]

In other words, this function flips `M` up to down and left to right.

good idea.

@kwankyu
Copy link
Collaborator

kwankyu commented May 22, 2017

comment:4

Looks and works nice. Thank you! Just two minor comments:

  1. Docstrings better to be consistent in the three methods. Note the period at the end.
Reverse the orders of rows and columns.
  1. You may want to also show the matrices before reversing in some of the examples.

@videlec
Copy link
Contributor Author

videlec commented May 22, 2017

comment:5

What do you think of calling the method simply reverse?

@kwankyu
Copy link
Collaborator

kwankyu commented May 22, 2017

comment:6

If someone implements reverse_rows or reverse_columns in future, then reverse will be somewhat unclear. I prefer to stick to the current name.

@videlec

This comment has been minimized.

@videlec
Copy link
Contributor Author

videlec commented May 22, 2017

comment:7

This method already exists and called antitranspose! I propose to keep this ticket to make an inplace version.

@videlec videlec changed the title method to reverse row and column orders of matrice Inplace version of antitranspose May 22, 2017
@videlec
Copy link
Contributor Author

videlec commented May 22, 2017

comment:8

Sorry, it is different: transpose and antitranspose does inverse rows <-> columns. They are symmetries with respect to diagonals. However, both transpose and antitranspose return a different matrix. It would be weird to have a different behavior for reverse.

I propose to have

  • transpose, antitranspose, reverse as methods returning a different matrix
  • _transpose_inplace, _antitranspose_inplace and _reverse_inplace that modifies the matrix

@videlec
Copy link
Contributor Author

videlec commented May 22, 2017

comment:9

Note that in numpy, there is method to do reflection that is called flip. What I am trying to achieve with this ticket corresponds to

sage: import numpy as np
sage: a = numpy.array([[0,1],[2,3],[4,5]])
sage: np.flip(np.flip(a,0), 1)
array([[5, 4],
       [3, 2],
       [1, 0]])

And numpy provides shortcuts flipud (up/down) and fliplr (left/right) for flipping with respect to axes 0 and 1.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 22, 2017

Changed commit from 17bbf4e to 39818b2

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 22, 2017

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

39818b223050: reverse_rows_and_columns

@videlec
Copy link
Contributor Author

videlec commented May 22, 2017

comment:11

Needs review again. Transpose and antitranspose are a bit different since the matrix space changes.

@videlec

This comment has been minimized.

@videlec videlec changed the title Inplace version of antitranspose method to reverse row and column orders of a matrix May 22, 2017
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 22, 2017

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

dbfaeaa23050: reverse_rows_and_columns

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 22, 2017

Changed commit from 39818b2 to dbfaeaa

@videlec

This comment has been minimized.

@kwankyu
Copy link
Collaborator

kwankyu commented May 22, 2017

comment:14

Still missing a period at the end:

Reverse the row order and column order of this matrix.

@kwankyu
Copy link
Collaborator

kwankyu commented May 22, 2017

comment:15

You may fold this long line

ValueError: matrix is immutable; please change a copy instead (i.e., use copy(M) to change a copy of M).

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 23, 2017

Changed commit from dbfaeaa to e7d56d0

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 23, 2017

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

e7d56d023050: documentation details

@videlec
Copy link
Contributor Author

videlec commented May 23, 2017

comment:17

done

@kwankyu
Copy link
Collaborator

kwankyu commented May 23, 2017

Reviewer: Kwankyu Lee

@vbraun
Copy link
Member

vbraun commented May 24, 2017

Changed branch from u/vdelecroix/23050 to e7d56d0

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