-
-
Notifications
You must be signed in to change notification settings - Fork 878
feat: add blas/ext/sorthp
#8098
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?
Conversation
Coverage Report
The above coverage report was generated for the changes in this PR. |
Signed-off-by: Muhammad Haris <[email protected]>
Signed-off-by: Muhammad Haris <[email protected]>
Signed-off-by: Athan <[email protected]>
Signed-off-by: Athan <[email protected]>
Signed-off-by: Athan <[email protected]>
The function has the following parameters: | ||
|
||
- **x**: input [ndarray][@stdlib/ndarray/ctor]. Must have a numeric or "generic" [data type][@stdlib/ndarray/dtypes]. | ||
- **sortOrder**: sort order (_optional_). May be either a scalar value or an [ndarray][@stdlib/ndarray/ctor] having a real or "generic" [data type][@stdlib/ndarray/dtypes]. If provided an [ndarray][@stdlib/ndarray/ctor], the value must have a shape which is [broadcast-compatible][@stdlib/ndarray/base/broadcast-shapes] with the complement of the shape defined by `options.dims`. For example, given the input shape `[2, 3, 4]` and `options.dims=[0]`, an [ndarray][@stdlib/ndarray/ctor] sort order must have a shape which is [broadcast-compatible][@stdlib/ndarray/base/broadcast-shapes] with the shape `[3, 4]`. Similarly, when performing the operation over all elements in a provided input [ndarray][@stdlib/ndarray/ctor], an [ndarray][@stdlib/ndarray/ctor] sort order must be a zero-dimensional [ndarray][@stdlib/ndarray/ctor]. By default, the sort order is `1` (i.e., increasing order). |
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.
I am wondering whether we should also support the string literals 'ascending'
and 'descending'
and then map those literals to 1
and -1
internally before calling into lower level functions. The rationale is that, from a code readability standpoint, a number literal does not readily convey its meaning. Compare
sorthp( x, -1 );
versus
sorthp( x, 'descending' );
The latter is more literate.
This comment is only directed at the top-level end-user APIs. I am not suggesting we update any lower-level packages.
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.
I am also okay supporting 'desc'
and 'asc'
, as well. Meaning, we could support four literal string values (but not ndarrays!): descending
, ascending
, desc
, and asc
.
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.
Although, the abbreviated desc
and asc
are a bit less obvious.
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.
@Planeshifter Do you have opinions here?
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.
@kgryte makes sense
Resolves stdlib-js/metr-issue-tracker#77.
Description
This pull request:
blas/ext/sorthp
Related Issues
This pull request:
blas/ext/sorthp
metr-issue-tracker#77Questions
No.
Other
No.
Checklist
@stdlib-js/reviewers