-
-
Notifications
You must be signed in to change notification settings - Fork 764
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
refactor: improve type definitions for ndarray array #1392
base: develop
Are you sure you want to change the base?
Conversation
@Planeshifter could you kindly review this PR? |
@@ -164,7 +164,7 @@ interface ExtendedOptions extends Options { | |||
* var v = arr.get( 0 ); | |||
* // returns [ 1, 2 ] | |||
*/ | |||
declare function array<T = unknown>( options: OptionsWithShape | OptionsWithBuffer ): typedndarray<T>; | |||
declare function array<T = unknown>( options: OptionsWithShape<T> | OptionsWithBuffer<T> ): typedndarray<typeof options['dtype'] extends undefined ? T : typeof options['dtype']>; |
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 not sure I follow how the return type is supposed to work. Can you provide examples of what you believe to be expected behavior?
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 also add corresponding tests to docs/types/test.ts
to confirm expected behavior. All our TypeScript declarations should have corresponding unit tests.
@@ -220,7 +220,7 @@ declare function array<T = unknown>( options: OptionsWithShape | OptionsWithBuff | |||
* var v = arr.get( 0, 0 ); | |||
* // returns 1.0 | |||
*/ | |||
declare function array<T = unknown>( buffer: ArrayLike<any>, options?: ExtendedOptions ): typedndarray<T>; | |||
declare function array<T = unknown>( buffer: ArrayLike<T>, options?: ExtendedOptions<T> ): typedndarray<T extends undefined ? ( typeof options extends { dtype: infer D } ? D : never ) : T >; |
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.
Same comments as above.
Resolves #1088
Description
This pull request:
array
function.dtype
's value, else usesbuffer
arguments' type.Related Issues
This pull request:
@stdlib/ndarray/array
#1088Questions
@Planeshifter From my understanding, I'm assuming the type of
buffer
argument and the type ofbuffer
property inoptions
argument are the same? Please let me know if otherwise.Other
No.
Checklist
@stdlib-js/reviewers