astype method lost its order parameter

[rhkleijn]

What happened:
I upgraded from xarray 0.15.1 to 0.16.2 and the astype method seems to have lost the order parameter.

In [1]: import xarray as xr

In [2]: xr.__version__
Out[2]: '0.16.2'

In [3]: xr.DataArray([[1.0, 2.0], [3.0, 4.0]]).astype(dtype='d', order='F').values.strides
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-3-208ab49008ef> in <module>
----> 1 xr.DataArray([[1.0, 2.0], [3.0, 4.0]]).astype(dtype='d', order='F').values.strides

TypeError: astype() got an unexpected keyword argument 'order'

What you expected to happen:
I was expecting to get the same result as with xarray 0.15.1:

In [1]: import xarray as xr

In [2]: xr.__version__
Out[2]: '0.15.1'

In [3]: xr.DataArray([[1.0, 2.0], [3.0, 4.0]]).astype(dtype='d', order='F').values.strides
Out[3]: (8, 16)

Anything else we need to know?:
Looking at the documentation it seems it disappeared between 0.16.0 and 0.16.1. The documentation at http://xarray.pydata.org/en/v0.16.0/generated/xarray.DataArray.astype.html
still has this snippet

order ({'C', 'F', 'A', 'K'}, optional) – Controls the memory layout order of the result. ‘C’ means C order, ‘F’ means Fortran order, ‘A’ means ‘F’ order if all the arrays are Fortran contiguous, ‘C’ order otherwise, and ‘K’ means as close to the order the array elements appear in memory as possible. Default is ‘K’.

(which was identical to the documentation from numpy.ndarray.astype at https://numpy.org/doc/stable/reference/generated/numpy.ndarray.astype.html)

while http://xarray.pydata.org/en/v0.16.1/generated/xarray.DataArray.astype.html seems to lack that part.

[mathause]

Must have been #4314

[dcherian]

Oops. @rhkleijn This would be a relatively easy fix if you have the time to send in a PR

[rhkleijn]

IIUC before PR #4314 numpy.ndarray.astype was used and the order parameter was just part of that. Looking through PR #4314 it seems that the 'casting' parameter required some special casing in duck_array_ops.py since not all duck arrays did support it (in particular sparse arrays only gained it very recently). In the current case the order parameter is not supported at all for sparse arrays.

I am thinking it might not even be worthwhile to add support for an order parameter similar to the numpy.ndarray.astype method:

• It would add more special casing logic to xarray.
• To the user it would result in either noisy warnings or silently dropping parameters.
• To me, the kind of contiguousness seems more like an implementation detail of the array type of the underlying data than as a property of the encapsulating xarray object.
• For my use case (with numpy arrays as the underlying data) I can quite easily work around this issue by using something like da.copy(data=np.asfortranarray(da.data)) for the example above (or np.ascontiguousarray for C contiguousness).

Another option might be to allow arbitrary **kwargs which will be passed through as-is to the astype method of the underlying array and making it the responsibility of the user to only supply parameters which make sense for that particular array type.

What are your thoughts? Does xarray have some kind of policy for supporting parameters which might not make sense for all types of duck arrays?

[dcherian]

@rhkleijn thanks for your thoughtful comment.

Does xarray have some kind of policy for supporting parameters which might not make sense for all types of duck arrays?

Not AFAIK but it would be good to have one.

Another option might be to allow arbitrary **kwargs which will be passed through as-is to the astype method of the underlying array and making it the responsibility of the user to only supply parameters which make sense for that particular array type.

I think this is a good policy.

Another option would be to copy the numpy signature with default None

def astype(dtype, order=None, casting=None, subok=None, copy=None)

and only forward kwargs that are not None. This has the advantage of surfacing all available parameters in xarray's documentation, but the default value would not be documented. It seems like only a small improvement over following your proposal and linking to numpy.ndarray.astype in the docstring

PS: The "special case" logic in astype will be removed, it was added as a temporary fix to preserve backward compatibility.

[rhkleijn]

That is a nicer solution. Never contributed a PR before, but I'll try to work on this next week. It looks like a good first issue.

[rhkleijn]

I will also include xarray's recently added keep_attrs argument. Since the signature has been in flux both in version 0.16.1 and now again I intend to make all arguments following dtype keyword-only in order to avoid introducing unnoticed bugs when user code calls it with positional arguments.

def astype(self, dtype, *, order=None, casting=None, subok=None, copy=None, keep_attrs=True)

[rhkleijn]

Working on a unit test for the subok argument I found no way for creating a DataArray backed by a subclass of np.ndarray (even with NEP 18 enabled).

The DataArray constructor calls xr.core.variable.as_compatible_data. This function:

• converts np.ma instances to np.ndarray with fill values applied.
• the NEP 18 specific branch which would keep the supplied data argument (and its type) is not reachable for (subclasses of) np.ndarray
• it converts these subclasses to np.ndarray.

Is this behaviour intentional?

What to do with astype? If the behaviour is not intentional we could add the subok unit test but marked as xfail until that is fixed. If it is intentional we could omit the subok argument here.