mathjs icon indicating copy to clipboard operation
mathjs copied to clipboard

Add methods for iteration of matrix rows and columns

Open cshaa opened this issue 4 years ago • 2 comments
trafficstars

Right now there is no comfortable way of doing something like this:

const mat = matrix([ ... ])

for (const col of mat.columns()) {
  console.log("here's a column: ", col)
}

The implementation of such a function would be very easy (using generators), but I'd like to discuss the design first.

Things we should address:

  • Is it needed at all? (I think it is, it makes working with matrices more comfy, as one doesn't need to touch mat._data)
  • Should it be implemented for Matrix only (as a mat.something method), or rather as a global method that works for arrays too?
  • What should it be called? Just columns is at risk for being misread as column (an already existing method), while iterateColumns feels awkward in a for..of (the code for (cons col of iterateColumns(mat)) doesn't read like a grammatical sentence). Maybe columnsOf(mat) and rowsOf(mat) could be better. Or maybe I'm just bikeshedding.

cshaa avatar Apr 12 '21 19:04 cshaa

This is an interesting idea, good to split this in a separate discussion/PR.

Just to repeat my initial thoughts in https://github.com/josdejong/mathjs/pull/2155#issuecomment-817104737:

Can you give some use cases for the new methods DenseMatrix.rows and DenseMatrix.columns? It is very close to the existing functions row and column. It introduces a new return type though which we discussed in the eigenvalues/vectors PR too: a nested array containig matrices. This may be useful methods but I want to be careful here since this construct may complicate matters. People will use the result of .rows() in other mathjs function like size(), but non of the mathjs functions understands this concept of an Array containing Matrices and will handle it expecting an Array containing Arrays, which probably gives weird/wrong/undefined behavior.

josdejong avatar Apr 18 '21 08:04 josdejong

Implementing this as an iterator instead of returning an Array with columns makes a lot of sense to me: then there is no conflict whatsoever with the current API.

An iterator is not currently usable in the expression parser, but there is no reason not be able to use it in the expression parser when implementing support for loops there, see #518, #2184.

josdejong avatar Apr 18 '21 08:04 josdejong

The motivation for introducing this feature will be reduced slightly by the adoption of #3036 solving #3014; should we close it as wontfix? Or if not, then I think it should at least be moved to a discussion, as it doesn't seem immediately actionable in any case.

gwhitney avatar Oct 03 '23 05:10 gwhitney

I understand what you mean though this is a more generic idea to have more options to iterate through rows/columns and do "some" matrix operation. Let's move this idea to the discussions section, we do not have concrete plans to implement it right now (and no much demand for it so far).

josdejong avatar Oct 04 '23 09:10 josdejong