-
-
Notifications
You must be signed in to change notification settings - Fork 872
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
base: develop
Are you sure you want to change the base?
feat: add ndarray/concat
#7969
Conversation
Need to merge #7853 first. |
* 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 |
There was a problem hiding this comment.
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.
N = arrays.length; | ||
arrs = []; | ||
if ( N < 1 ) { | ||
throw new RangeError( format( 'invalid argument. First argument must have more than one ndarray. Value: `%s`.', N ) ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 ) ); |
* @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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs updating.
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 ] ) ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 ] ) ); |
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 ] ) { |
There was a problem hiding this comment.
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.
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 ) { |
There was a problem hiding this comment.
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?
} | ||
ord = resolveOutputOrder( orders ); | ||
if ( out ) { | ||
if ( getDtype( out ) !== dt ) { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
/stdlib merge |
Coverage Report
The above coverage report was generated for the changes in this PR. |
Progresses #2656.
Description
This pull request:
ndarray/concat
Related Issues
This pull request:
Questions
No.
Other
No.
Checklist
@stdlib-js/reviewers