-
Notifications
You must be signed in to change notification settings - Fork 52
Add specifications for array creation functions #31
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
rgommers
left a comment
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.
Most choices for the inconsistencies you flagged seem fine to me.
linspace: Torch and TensorFlow don't support endpoint keyword arg.
Including endpoint in the signature here is the one choice I'd probably make differently. It's rarely used I think, and if both PyTorch and TensorFlow don't have the keyword, it may be better to not include it.
|
@rgommers Thanks for the review! Re: linspace Re: |
This is addressed in gh-32
Thanks. Okay, I agree with keeping it. Nice to have data:)
Yeah, I think the ship has sailed, too heavily used so can't change it. Also, it's also consistent with the builtin |
|
@kgryte what's here is ready right? PR says WIP, but that's because you wanted to add more functions? Maybe we should merge this as is soon, and in a second PR list add (if needed) other array creation functions as well as list the ones that were left out. |
…o array-creation
|
@rgommers Correct. Should be ready. I've updated the And yes, there are a few additional APIs to add. The main one being |
rgommers
left a comment
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.
Latest update LGTM, in it goes. Thanks @kgryte
This PR
Notes
This list of array creation functions is an initial set of array creation functions which can pave the way for additional specs in subsequent pull requests. These functions were identified as the set of functions with the broadest support among array libraries and relatively higher usage among downstream libraries.
Some comments regarding particular APIs...
eye: Torch and TensorFlow don't currently support
kkeyword argument for off-diagonal.full: TensorFlow doesn't support
dtype. Infers fromfill_value.full_like: Torch and MXNet support an
outkeyword argument.linspace: Torch and TensorFlow don't support returning the step along with the samples. TensorFlow infers
dtypefromstart. Torch and TensorFlow don't supportendpointkeyword arg. Torch/CuPy/dask.array does not supportaxiskeyword arg.ones: NumPy default
dtypeisfloat64, while TensorFlow isfloat32.ones_like:
shapekeyword argument for overriding shape is not universal and seems mostly intended for generating an array having the samedtype, but not the same shape.Note: some of the comments apply to multiple APIs (e.g.,
ones,zerosandones_like,zeros_like, etc.Questions
dtype?