-
-
Notifications
You must be signed in to change notification settings - Fork 835
feat: support negative strides for idamax
and isamax
#2793
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
Conversation
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.
@Pranavchiku We also need to update the ndarray.native.js
bindings. We shouldn't have divergent behavior if the implementation is JS or via a native add-on. The change to ndarray.native.js
is straightforward, and we need to update the tests accordingly.
idamax
and isamax
idamax
and isamax
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.
@Pranavchiku Looks good. Only small change is that the notes in the README need to be updated. Namely,
- If `N < 1`, both functions return `-1`.
...and in the REPL text file.
Done, this PR can have a final review. |
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.
LGTM. Thanks, @Pranavchiku!
PR-URL: stdlib-js#2793 Closes: stdlib-js#2792 Ref: stdlib-js#2464 Reviewed-by: Athan Reines <kgryte@gmail.com>
Resolves #2792. Towards #2464.
Description
This pull request enhances existing JS ndarray implementation of
blas/base/idamax
andblas/base/isamax
to support negative strides.Related Issues
NA
Questions
No.
Other
No.
Checklist
@stdlib-js/reviewers