Update API specification for creation and manipulation functions #167
Conversation
|
|
||
| - **dtype**: _Optional\[ <dtype> ]_ | ||
|
|
||
| - output array data type. If `dtype` is `None`, the output array data type must be the default floating-point data type. Default: `None`. | ||
| - output array data type. If `dtype` is `None`, the output array data type must be inferred from `start`, `stop` and `step`. If those are all integers, the output array dtype must be the default integer dtype; if one or more have type `float`, then the output array dtype must be the default floating-point data type. Default: `None`. |
There was a problem hiding this comment.
Does the spec talk about "default integer dtype" anywhere? It says
A conforming implementation of the array API standard must define a default floating-point data type (either
float32orfloat64), as well as a default data type for an array index (eitherint32orint64).
There was a problem hiding this comment.
It does, in the "data types" section. My last commit extends that paragraph.
There was a problem hiding this comment.
Meh, wording - that was intended as "default integer dtype" I believe.
There was a problem hiding this comment.
Updated that section. A default dtype for indexing arrays doesn't make sense to me; only the values indexing accepts are meaningful (can it exceed 32-bit). But given that we don't have advanced indexing, even that doesn't really make sense. Why do we need this at all?
There was a problem hiding this comment.
This is what take can be used for, but we don't have it. On the other hand, if we'd be able to add take, why not just add 1-D integer array indexing?
There was a problem hiding this comment.
It would be easier to use take to be able to mark it as problematic for shape inference. Worth considering.
There was a problem hiding this comment.
It's also relevant for integer indexing. A 32-bit integer index type limits the max size of a dimension.
There was a problem hiding this comment.
That's more a "how large can your array be" limitation than an indexing thing though.
|
From a grep, |
rgommers
force-pushed
the
creation-funcs
branch
from
d7b8738 to
ba024b9
Compare
Addresses comments in data-apisgh-85 and data-apisgh-107
Addresses comments in data-apisgh-85 and data-apisgh-107
Addresses comments in data-apisgh-85 and data-apisgh-107
Addresses comments in data-apisgh-85 and data-apisgh-107
This is useful/needed because `M` is not a descriptive name and that name does not match between different array libraries.
Address comment in data-apisgh-85
rgommers
force-pushed
the
creation-funcs
branch
from
ba024b9 to
271cbb7
Compare
Thanks, included now. |
Address a review comment; makes it consistent with other functions using `shape`.
rgommers
force-pushed
the
creation-funcs
branch
from
271cbb7 to
4050990
Compare
rgommers
added
API change
|
LGTM. Let's resolve the conflict. |
|
Thanks Leo. Okay, let's get this in then |
Addresses a lot of detailed comments, mostly on function signatures and default dtypes.
Closes gh-85
Closes gh-107
Closes gh-151