-
Notifications
You must be signed in to change notification settings - Fork 53
Issues with array creation functions #107
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
Comments
Thanks @asmeurer, good points. I agree with everything except the |
Re: Re: Re: Re: other points. Seem reasonable. Will submit a follow-up PR. |
Just to be clear, the spec doesn't explicitly say the endpoint should be exactly included. So if that is desired, we should probably state that.
I'm not sure I follow here. I mean the case where >>> np.arange(9223372036854775805, 9223372036854775807, dtype=np.float64)
array([9.22337204e+18, 9.22337204e+18])
>>> np.unique(np.arange(9223372036854775805, 9223372036854775807, dtype=np.float64))
array([9.22337204e+18] |
A few other suggestions:
|
Could these libraries simply alias Also, a common use of |
Fair enough -- in fact this is exactly what JAX already does. |
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
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
* Update specification for arange Addresses comments in gh-85 and gh-107 * Update the specification for `full` and `full_like` Addresses comments in gh-85 and gh-107 * Update specification for `linspace` Addresses comments in gh-85 and gh-107 * Update specification for `empty`, `ones`, `zeros` Addresses comments in gh-85 and gh-107 * Update specification for `eye` This is useful/needed because `M` is not a descriptive name and that name does not match between different array libraries. * Update specification for `expand_dims`, `roll` and `reshape` Address comment in gh-85 * One more change to `eye`, more descriptive positional arguments * Address the default integer dtype issue for 32/64-bit Python Closes gh-151 * Update signature of `broadcast_to` Address a review comment; makes it consistent with other functions using `shape`.
Turns out the linspace issue isn't just about the endpoint: >>> np.linspace(-9007199254740993, 0, 1, dtype=np.int64)
array([-9007199254740992]) I think this is related to this NumPy issue numpy/numpy#16813. |
The spec is maybe a little unclear, but I read it as saying the |
That looks like a clear bug, not even a subtle one. The integer start point and step size calculation should just not use floats. |
Some issues I noticed in the array creation functions from adding tests to the test suite:
arange
dtype
isNone
, the output array data type must be the default floating-point data type." I think the default for arange should be int if all the arguments are integers.step
cannot be 0.ceil((stop-start)/step)
" should be caveated (stop and step provided, stop >= start for step > 0 and stop <= start for step < 0)eye
full (and full_like)
dtype
isNone
, the output array data type must be the default floating-point data type." I think the default should be a corresponding dtype to the input value (we don't have a notion of a "default" integer dtype).linspace
The stop value is different from what is given because of floating point loss of precision when computing the linspace.
The text was updated successfully, but these errors were encountered: