Repository navigation
BUG: Array API reshape does not conform to standard #23410
Description
Describe the issue:
The reshape manipulation function in the new Array API takes a copy argument that we don't replicate in NumPy.
I think this is unintentional
Reproduce the code example:
import numpy.array_api as nx
array = nx.arange(10, dtype=nx.int64)
other = nx.reshape(array, (2, 5), copy=True)
array[:5] = -1
print(other)Error message:
/tmp/test.py:1: UserWarning: The numpy.array_api submodule is still experimental. See NEP 47.
import numpy.array_api as nx
Traceback (most recent call last):
File "/tmp/test.py", line 4, in <module>
other = nx.reshape(array, (2, 5), copy=True)
TypeError: reshape() got an unexpected keyword argument 'copy'Runtime information:
1.23.5
3.10.9 | packaged by conda-forge | (main, Feb 2 2023, 20:20:04) [GCC 11.3.0]
Context for the issue:
No response
Activity
agoose77 commentedon Mar 16, 2023
@asmeurer :) I can't make a PR just yet, but here's the issue :)
asmeurer commentedon Mar 16, 2023
Yes, anything in the 2021.12 standard should be implemented in np.array_api. This was just missed.
bwalshe commentedon Apr 18, 2023
I had a quick go at cleaning this up, but I am a little unclear what the semantics of "copy" means here. Does it just refer to the underlying array data, or does it refer to the array object? Like if
copy=FalseorNonedoes that mean that the shape of the input array object should be mutated?I have an initial commit here. It looks like there were a few missing unit tests for the array manipulation functions, so I added them. I only tested for things that were different between Numpy and the Array API, as it looks like that is what is being done elsewhere. Is that alright.
Also, it looks like copying isn't fully implemented for
asarrayeither. I assume it would be best to fix that separately to fixing this?agoose77 commentedon Apr 18, 2023
@bwalshe conventionally
copyonly refers to array data. I don't think there's any statement regarding the array object itself. I'd assume this is free to e.g. return the same object if e.g. nothing needs to change. However, I would expectcopy=Falseto always return a new object if the shape does change; we don't want to mutate the array.asmeurer commentedon May 15, 2023
That's maybe something that could be clarified in the standard, but
copyshould refer to the data. There's no real requirements, as far as the array API spec goes, about the actual array container object.In numpy.array_api in particular, there's two objects, the wrapper
Arrayobject and the underlyingndarray. I think there might already be some places where we create a new wrapper object even if we don't have to. I would go with whatever implementation is best for performance, i.e., avoid creating a new object if it is unnecessary.The test suite in numpy.array_api only focuses on things that aren't already tested by array-api-tests. This primarily means strictness things (i.e., mostly exceptions), because the spec doesn't require strictness so the array-api test suite doesn't check for it, but we do want numpy.array_api to be strict/minimal. Your tests look fine to me. The actual reshape tests themselves aren't strictly needed if you consider array-api-tests, but all the
raiseschecks are things that aren't checked by the test suite.Can you open a PR with your fix?
BUG: add copy parameter for api.reshape function
BUG: add copy parameter for api.reshape function
bwalshe commentedon May 22, 2023
@asmeurer I feel this fix is still a wip, and may need some changes to the C code to get working the way I would like. I have not had a chance to look into this in a while, but I have opened a PR with the changes I have made so far. I might need a bit of guidance to get this closed.
BUG: add copy parameter for api.reshape function
rgommers commentedon Feb 29, 2024
It looks like this was fixed in gh-23789 for 1.26.x and backported for 1.25.0 in gh-23957, so I'll close the issue. Thanks all!