Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Add methods for iteration of matrix rows and columns #2177

Closed
cshaa opened this issue Apr 12, 2021 · 4 comments
Closed

Add methods for iteration of matrix rows and columns #2177

cshaa opened this issue Apr 12, 2021 · 4 comments
Labels

Comments

@cshaa
Copy link
Collaborator

cshaa commented Apr 12, 2021

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.
@josdejong
Copy link
Owner

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

Just to repeat my initial thoughts in #2155 (comment):

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
Copy link
Owner

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.

@gwhitney
Copy link
Collaborator

gwhitney commented Oct 3, 2023

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.

@josdejong
Copy link
Owner

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).

Repository owner locked and limited conversation to collaborators Oct 4, 2023
@josdejong josdejong converted this issue into discussion #3056 Oct 4, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
Projects
None yet
Development

No branches or pull requests

3 participants