Expected behaviour regarding new copy keyword in __array__ implementations

@jorisvandenbossche

Moving from #25922, which added this to the docs:

NumPy will pass copy to the __array__ special method in situations where
it would be set to a non-default value (e.g. in a call to
np.asarray(some_object, copy=False)). Currently, if an
unexpected keyword argument error is raised after this, NumPy will print a
warning and re-try without the copy keyword argument. Implementations of
objects implementing the __array__ protocol should accept a copy keyword
argument with the same meaning as when passed to numpy.array or
numpy.asarray.

Thinking through how to update some __array__ implementations, I am wondering: what are the expectations from numpy how an __array__ implementations handles this keyword fully or partially?

When copy=True is being passed, does numpy assume that the __array__ implementation already made that copy? But what if the implementation defers dtype casting to numpy (which is currently possible), then it might be a waste to already copy up front?

Originally posted by @jorisvandenbossche in #25922 (comment)

Deprecated though, right? The signature should already have included the dtype keyword, even though it was still working to not have that.

then it might be a waste to already copy up front?

What we want to achieve I think is that only a single copy is ever made. We can have two now in many cases, because the other library may copy from its internal implementation into an array, and then numpy will make another copy. The only way to get it right is for the other library to do the copy if copy=True, and for numpy not to do so if the method call including copy=True succeeded.

The old behavior is:

the only contract of __array__ is that it must return a numpy.ndarray instance,
the implementer of __array__ may or may not make a copy in the process of producing an array
NumPy may make a copy of the returned array if needed when copy=False, and must make a copy if copy=True (it cannot know otherwise if it has exclusive ownership of the underlying data)
end result for copy=True is: either 1 or 2 copies made (NumPy cannot know which)

The intended new behavior is:

copy is always passed on to the __array__ implementer, with the same semantics as for asarray (False: "never copy", None: "copy-if-needed", True: ensure data is a semantic copy / unique to returned array)
that implementer must ensure that the data can not be used anymore by anything other than the numpy array it's returning (that usually means "copy the data in memory" but it can also mean something like "invalidate any other reference to this data")
end result:
- copy=False: guarantee that no data is copied in memory
- copy=None: 0 or 1 copies made, data may be a view
- copy=True: 0 or 1 copies made, data is guaranteed to not be a view

It looks like we need to do two things:

Make the expectations on implementers of __array__ clearer in our docs,
Ensure we actually do not make a second copy if copy=True if passing along the copy keyword in the call to __array__ succeeded.

This change of __array__ kinda came in at the end of a long PR/discussion about the asarray/array behavior, so I didn't realize until now that this is actually also a performance improvement, potentially avoiding an unnecessary copy.

One additional aspect of the old behaviour is illustrated as follows:

class MyArray:
    def __init__(self, arr):
        self._ndarray = arr

    def __array__(self, dtype=None):
        return self._ndarray

In [7]: arr = MyArray(np.array([1, 2, 3]))

In [8]: np.asarray(arr, dtype="float64")
Out[8]: array([1., 2., 3.])

So when __array__ returns an np.ndarray that doesn't match the requested dtype, then numpy does the casting. In all of pandas / geopandas / shapely, I know we have some __array__ implementations that do this.

When a copy keyword is added, I assume we can't keep such a simple implementation if we want to correctly honor the keyword. Numpy will now assume the library did correctly honor the keyword? For example, with the above code, when doing np.asarray(arr, dtype="int64", copy=True), i.e. with a matching dtype, numpy doesn't need to do a cast and thus not need to copy, and then will assume that copy already happened?

It seems that on current main, numpy still does a copy afterwards, so np.asarray(arr, dtype="int64", copy=True) actually gives a copy even with the __array__ implementation above. But I assume numpy will want to change that to avoid the additional copy?

But I assume numpy will want to change that to avoid the additional copy?

Right now it's actually not possible to get to the code path in numpy that calls __array__ with copy=True, but in the future we'll want to restructure things to avoid the extra copy.

This should probably be fixed:

import numpy as np

class HasArray:
    def __array__(self, values=None, dtype=None, copy=None):
        print(copy)
        return

arr = HasArray()

np.array(arr, copy=True)

Right now the print(copy) will print None where it really should print True.

We also need to update the docs to cover how implementers should handle this.

Added test cases for the new __array__ methods(Copy=true/false/None)

Closing, thanks.

Added test cases for the new __array__ methods(Copy=true/false/None)

I have an additional question here that is not fully covered in the migration guide note that closed this issue:

For any __array__ method on a non-NumPy array-like object, a copy=None keyword can be added to the signature - this will work with older NumPy versions as well. If copy keyword is considered in the __array__ method implementation, then for copy=True always return a new copy.

I don't know what "if copy keyword is considered" means exactly (only when handling the copy keyword in your actual implementation, or whenever adding the copy keyword to the signature?), but it seems that adding copy=None to your __array__ implementation actually does impact the behaviour.

# no copy keyword -> raises an error (and depr warning) when specifying copy=False

class MyArray:
    def __array__(self, dtype=None):
        return np.array([1,2,3], dtype=int)

>>> np.array(MyArray(), copy=False)
DeprecationWarning: __array__ should implement the 'dtype' and 'copy' keyword argument
ValueError: Unable to avoid copy while creating an array as requested.
If using `np.array(obj, copy=False)` replace it with `np.asarray(obj)` to allow a copy when needed (no behavior change in NumPy 1.x).
For more details, see https://numpy.org/devdocs/numpy_2_0_migration_guide.html#adapting-to-changes-in-the-copy-keyword.

# adding copy keyword just to signature -> no error

class MyArray:
    def __array__(self, dtype=None, copy=None):
        return np.array([1,2,3], dtype=int)

>>> np.array(MyArray(), copy=False)
array([1, 2, 3])

So just adding copy=None to the signature but further ignoring it (to get rid of the deprecation warning) results in np.array(.., copy=False) to no longer raise an error, while there is no guarantee that the result is not copied?

And this is what for example pandas did (pandas-dev/pandas#57739, and I just wanted to do the same for pyarrow).

Is the behaviour difference expected? (also in the case of copy=False that is now raising, the __array__ method might actually be returning something that is not a copy, and thus numpy can be raising an incorrect error)
And should the recommendation then be to only add copy=None to your __array__ signature when actually honoring the keyword, also when it is passed copy=False? (the current doc note only mentions that it should honor copy=True)

@mtsokol

So just adding copy=None to the signature but further ignoring it (to get rid of the deprecation warning) results in np.array(.., copy=False) to no longer raise an error, while there is no guarantee that the result is not copied?

This is working as intended. We only raise the warning if the function doesn't support the keyword at all. As soon as you add the keyword (even if you ignore it), numpy has to assume the implementation has done what the user expects when copy=False is passed. It would be nice if we could do something fancier and actually check if a copy happened or not, but I don't think there's a way to tell from inside numpy?

(also in the case of copy=False that is now raising, the array method might actually be returning something that is not a copy, and thus numpy can be raising an incorrect error)

Hmm, maybe it would be better to reword the error message to say something like "numpy tried passing copy=False, but the __array__ implementation doesn't accept a copy keyword, so numpy can't guarantee that a copy won't be made".

Ping @mtsokol, can we get some minor updates to docs and error messages to alleviate the confusion @jorisvandenbossche ran into?

Hmm, maybe it would be better to reword the error message to say something like "numpy tried passing copy=False, but the __array__ implementation doesn't accept a copy keyword, so numpy can't guarantee that a copy won't be made".

That message would have helped me as implementor, but is probably not super useful for users? (they don't control the implementation of __array__ and might not even be aware that this is how the conversion happens)

Update the migration guide / __array__ docs specifying in more detail how to handle the copy keyword would be useful!

If we touch the docs, it would be nice to use "must" rather than "should". Should can (and when it comes to such things does) convey that downstream has a choice: they don't (i.e. is more like a strong recommendation)! So the correct terminology is "must".

Another point of feedback: for implementing the copy=False case correctly, it is unfortunate (for this specific case, not necessarily for general usage!) that the copy keyword of astype is not strict.
Because we have a (typical?) usage of something like:

class MyArray:

    def __array__(self, dtype=None):
            if dtype is None:
                return self._ndarray
            return self._ndarray.astype(dtype)

To update this to have a copy=None keyword ànd honor the copy=False case, it's not sufficient to add astype(dtype, copy=False) as that still doesn't guarantee no copy was made. Once would have the return value of astype to check if it returned the calling ndarray as is, to detect if a copy happened or not?

EDIT: on second thought, I suppose this can be done much simpler by actually relying on the new behaviour of np.array and its copy keyword that is now strict, by just doing return np.array(self._ndarray, dtype=dtype, copy=False)?

The copy keyword for astype is mentioned in NEP 56, where disallowing copies with copy=False is ruled out because astype implies a user explicitly requests a different dtype and a copy has to happen if the dtype changes. See also #25925.

From your edit, it sounds like you figured out an appropriate workaround.

@jorisvandenbossche

@jorisvandenbossche I think this could be the shortest way to support both dtype and copy:

def __array__(self, dtype=None, copy=None):
    dtype = self._ndarray.dtype if dtype is None else dtype
    return np.array(self._ndarray, dtype=dtype, copy=copy)

Added test cases for the new __array__ methods(Copy=true/false/None)

GH-41098: [Python] Add copy keyword in Array.__array__ for numpy 2.0+…

This has been explained, and a new PR is open to change the behavior, but it will be in 2.1 with a deprecation. Closing this.

apacheGH-41098: [Python] Add copy keyword in Array.__array__ for nump…

Can this comment #25941 (comment) be linked to in the deprecation warning?

it was very useful to me. I can make the PR if ok!

Do you mean adding a link to the comment? Most of it has been put into the migration guide, no?

array creation needs to have copy argument

Navigation Menu

Repository navigation

Expected behaviour regarding new `copy` keyword in `array` implementations #25941

1 remaining item

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Expected behaviour regarding new copy keyword in __array__ implementations #25941

Description

Activity

rgommers commented on Mar 5, 2024

jorisvandenbossche commented on Mar 5, 2024

jorisvandenbossche commented on Mar 5, 2024

ngoldbaum commented on Mar 5, 2024

ngoldbaum commented on Mar 20, 2024

mattip commented on Apr 3, 2024

jorisvandenbossche commented on Apr 8, 2024

ngoldbaum commented on Apr 8, 2024

jorisvandenbossche commented on Apr 8, 2024

1 remaining item

seberg commented on Apr 8, 2024

jorisvandenbossche commented on Apr 9, 2024

ngoldbaum commented on Apr 9, 2024

mtsokol commented on Apr 9, 2024

seberg commented on Apr 17, 2024

hmaarrfk commented on Sep 8, 2024

mattip commented on Sep 8, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Expected behaviour regarding new `copy` keyword in `array` implementations #25941