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

feat: add matrixGetSubMatrix function #236

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Conversation

jobo322
Copy link
Member

@jobo322 jobo322 commented Mar 21, 2024

No description provided.

@jobo322 jobo322 requested a review from targos March 21, 2024 21:02
src/matrix/__tests__/matrixCheckRanges.test.ts Outdated Show resolved Hide resolved
src/matrix/__tests__/matrixGetSubMatrix.test.ts Outdated Show resolved Hide resolved
src/matrix/matrixGetSubMatrix.ts Outdated Show resolved Hide resolved
@targos
Copy link
Member

targos commented Mar 22, 2024

Please run eslint-fix and make sure your editor runs eslint on save.

src/matrix/index.ts Outdated Show resolved Hide resolved
src/matrix/matrixGetSubMatrix.ts Outdated Show resolved Hide resolved
src/matrix/matrixGetSubMatrix.ts Outdated Show resolved Hide resolved
const nbColumns = endColumn - startColumn + 1;
const nbRows = endRow - startRow + 1;

const subMatrix = matrixCreateEmpty({ nbColumns, nbRows });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if we should not simply use ml-matrix but anyway this seems like a very slow method.
Why create an empty matrix ?
Just make a loop on the selected rows and push a 'slice'.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder also where it is being used because if we create temporary submatrix for some applications we can always create a 'subarray' https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray/subarray that is a pointer to the original array (must be typed). It is very specific but we don't duplicate the data. We can discuss on telegram if it can be used in your application.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants