feat: add `ndarray/concat` #7969

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

Jump to bottom

Draft

headlessNode wants to merge 2 commits into stdlib-js:develop

from headlessNode:ndarray-concat

Draft

feat: add `ndarray/concat` #7969

headlessNode wants to merge 2 commits into stdlib-js:develop from headlessNode:ndarray-concat

Conversation

headlessNode

Copy link

Member

@headlessNode headlessNode commented Aug 25, 2025

Progresses #2656.

Description

What is the purpose of this pull request?

This pull request:

add ndarray/concat

Related Issues

Does this pull request have any related issues?

This pull request:

progresses [RFC]: achieve ndarray API parity with built-in JavaScript arrays #2656

Questions

Any questions for reviewers of this pull request?

No.

Other

Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.

No.

Checklist

Please ensure the following tasks are completed before submitting this pull request.

Read, understood, and followed the contributing guidelines.

@stdlib-js/reviewers

@headlessNode


 feat: add ndarray/concat

f375988

@headlessNode headlessNode added Feature GSoC gsoc: 2025 labels

Aug 25, 2025

@headlessNode

Copy link

Member Author

headlessNode commented Aug 25, 2025

Need to merge #7853 first.

@headlessNode headlessNode mentioned this pull request

Aug 26, 2025

Weekly Standup (2025年08月25日) stdlib-js/google-summer-of-code#168

Closed

kgryte

kgryte reviewed

Sep 7, 2025

View reviewed changes

lib/node_modules/@stdlib/ndarray/concat/lib/main.js

* Concatenates a list of ndarrays along a specified ndarray dimension.

* @param {ArrayLikeObject<Object>} arrays - array-like object containing input ndarrays

* @param {integer} dim - dimension along which the arrays are concatenated

Copy link

Member

@kgryte kgryte Sep 7, 2025

Choose a reason for hiding this comment

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

This needs to be a negative integer and should be validated as such.

kgryte

kgryte reviewed

Sep 7, 2025

View reviewed changes

lib/node_modules/@stdlib/ndarray/concat/lib/main.js

N = arrays.length;

arrs = [];

if ( N < 1 ) {

throw new RangeError( format( 'invalid argument. First argument must have more than one ndarray. Value: `%s`.', N ) );

Copy link

Member

@kgryte kgryte Sep 7, 2025

Choose a reason for hiding this comment

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

Suggested change

throw new RangeError( format( 'invalid argument. First argument must have more than one ndarray. Value: `%s`.', N ) );

throw new RangeError( format( 'invalid argument. First argument must have one or more ndarrays. Value: `%s`.', N ) );

kgryte

kgryte reviewed

Sep 7, 2025

View reviewed changes

lib/node_modules/@stdlib/ndarray/concat/lib/main.js

* @param {ArrayLikeObject<Object>} arrays - array-like object containing input ndarrays

* @param {integer} dim - dimension along which the arrays are concatenated

* @throws {TypeError} first argument must be an array of ndarray-like objects

* @throws {RangeError} first argument must have more than one ndarray

Copy link

Member

@kgryte kgryte Sep 7, 2025

Choose a reason for hiding this comment

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

Needs updating.

kgryte

kgryte reviewed

Sep 7, 2025

View reviewed changes

lib/node_modules/@stdlib/ndarray/concat/lib/main.js

orders = [];

for ( i = 0; i < N; i++ ) {

if ( !isndarrayLike( arrays[ i ] ) ) {

throw new TypeError( format( 'invalid argument. First argument must be an array of ndarray-like objects. Value: `%s`.', arrays[ i ] ) );

Copy link

Member

@kgryte kgryte Sep 7, 2025

Choose a reason for hiding this comment

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

Suggested change

throw new TypeError( format( 'invalid argument. First argument must be an array of ndarray-like objects. Value: `%s`.', arrays[ i ] ) );

throw new TypeError( format( 'invalid argument. First argument must be an array of ndarrays. Value: `%s`.', arrays[ i ] ) );

kgryte

kgryte reviewed

Sep 7, 2025

View reviewed changes

lib/node_modules/@stdlib/ndarray/concat/lib/output.js

throw new RangeError( format( 'invalid argument. Output ndarray must have %d dimensions. Value: %d.', sh.length, getShape( out ).length ) );

}

for ( i = 0; i < sh.length; i++ ) {

if ( getShape( out )[ i ] !== sh[ i ] ) {

Copy link

Member

@kgryte kgryte Sep 7, 2025

Choose a reason for hiding this comment

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

You should not be calling getShape repeatedly like this. You're materializing a new shape array for every call.

kgryte

kgryte reviewed

Sep 7, 2025

View reviewed changes

lib/node_modules/@stdlib/ndarray/concat/lib/output.js

if ( getDtype( out ) !== dt ) {

throw new TypeError( format( 'invalid argument. Output ndarray must have a dtype of `%s`. Value: `%s`.', dt, getDtype( out ) ) );

}

if ( getOrder( out ) !== ord ) {

Copy link

Member

@kgryte kgryte Sep 7, 2025

Choose a reason for hiding this comment

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

Why is it necessary for the output array to have the resolved order?

kgryte

kgryte reviewed

Sep 7, 2025

View reviewed changes

lib/node_modules/@stdlib/ndarray/concat/lib/output.js

}

ord = resolveOutputOrder( orders );

if ( out ) {

if ( getDtype( out ) !== dt ) {

Copy link

Member

@kgryte kgryte Sep 7, 2025

Choose a reason for hiding this comment

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

Why is it necessary for the output array to have the promoted data type?

Copy link

Member

@kgryte kgryte Sep 7, 2025

Choose a reason for hiding this comment

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

What about mostly safe casts?

@kgryte

Copy link

Member

kgryte commented Sep 7, 2025

/stdlib merge

@stdlib-bot stdlib-bot added the bot: In Progress label

Sep 7, 2025

@stdlib-bot


 Merge remote-tracking branch 'upstream/develop' into ndarray-concat

@stdlib-bot stdlib-bot removed the bot: In Progress label

Sep 7, 2025

@stdlib-bot

Copy link

Contributor

stdlib-bot commented Sep 7, 2025

Coverage Report

Package	Statements	Branches	Functions	Lines
ndarray/concat	$\color{red}271/467$ $\color{green}+58.03\%$	$\color{green}4/4$ $\color{green}+100.00\%$	$\color{red}0/4$ $\color{green}+0.00\%$	$\color{red}271/467$ $\color{green}+58.03\%$

The above coverage report was generated for the changes in this PR.

Labels

Feature GSoC gsoc: 2025

3 participants

@headlessNode @kgryte @stdlib-bot

Uh oh!

feat: add ndarray/concat #7969

Are you sure you want to change the base?

feat: add ndarray/concat #7969

Uh oh!

Conversation

@headlessNode headlessNode commented Aug 25, 2025

Description

Related Issues

Questions

Other

Checklist

Uh oh!

headlessNode commented Aug 25, 2025

Uh oh!

@kgryte kgryte Sep 7, 2025

Choose a reason for hiding this comment

Uh oh!

@kgryte kgryte Sep 7, 2025

Choose a reason for hiding this comment

Uh oh!

@kgryte kgryte Sep 7, 2025

Choose a reason for hiding this comment

Uh oh!

@kgryte kgryte Sep 7, 2025

Choose a reason for hiding this comment

Uh oh!

@kgryte kgryte Sep 7, 2025

Choose a reason for hiding this comment

Uh oh!

@kgryte kgryte Sep 7, 2025

Choose a reason for hiding this comment

Uh oh!

@kgryte kgryte Sep 7, 2025

Choose a reason for hiding this comment

Uh oh!

@kgryte kgryte Sep 7, 2025

Choose a reason for hiding this comment

Uh oh!

kgryte commented Sep 7, 2025

Uh oh!

stdlib-bot commented Sep 7, 2025

Coverage Report

Uh oh!

Uh oh!

feat: add `ndarray/concat` #7969

feat: add `ndarray/concat` #7969