Why does xr.apply_ufunc support numpy/dask.arrays?
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 4.2k
- Forks
- 1.4k
- Avg merge
- 2d 15h
- Merged PRs (30d)
- 14
Description
What is your issue?
@keewis pointed out that it's weird that xarray.apply_ufunc supports passing numpy/dask arrays directly, and I'm inclined to agree. I don't understand why we do, and think we should consider removing that feature.
Two arguments in favour of removing it:
- It exposes users to transposition errors
Consider this example:
In [1]: import xarray as xr
In [2]: import numpy as np
In [3]: arr = np.arange(12).reshape(3, 4)
In [4]: def mean(obj, dim):
...: # note: apply always moves core dimensions to the end
...: return xr.apply_ufunc(
...: np.mean, obj, input_core_dims=[[dim]], kwargs={"axis": -1}
...: )
...:
In [5]: mean(arr, dim='time')
Out[5]: array([1.5, 5.5, 9.5])
In [6]: mean(arr.T, dim='time')
Out[6]: array([4., 5., 6., 7.])
Transposing the input leads to a different result, with the value of the dim kwarg effectively ignored. This kind of error is what xarray code is supposed to prevent by design.
- There is an alternative input pattern that doesn't require accepting bare arrays
Instead, any numpy/dask array can just be wrapped up into an xarray Variable/NamedArray before passing it to apply_ufunc.
In [7]: from xarray.core.variable import Variable
In [8]: var = Variable(data=arr, dims=['time', 'space'])
In [9]: mean(var, dim='time')
Out[9]:
<xarray.Variable (space: 4)> Size: 32B
array([4., 5., 6., 7.])
In [10]: mean(var.T, dim='time')
Out[10]:
<xarray.Variable (space: 4)> Size: 32B
array([4., 5., 6., 7.])
This now guards against the transposition error, and puts the onus on the user to be clear about which axes of their array correspond to which dimension.
With Variable/NamedArray as public API, this latter pattern can handle every case that passing bare arrays in could.
I suggest we deprecate accepting bare arrays in favour of having users wrap them in Variable/NamedArray/DataArray objects instead.
(Note 1: We also accept raw scalars, but this doesn't expose anyone to transposition errors.)
(Note 2: In a quick scan of the apply_ufunc docstring, the docs on it in computation.rst, and the extensive guide that @dcherian wrote in the xarray tutorial repository, I can't see any examples that actually pass bare arrays to apply_ufunc.)
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start at the xarray.apply_ufunc entry point and review its documentation, computation.rst, and the tutorial guidance mentioned in the issue. Determine the deprecation or removal scope for bare NumPy and Dask arrays, while preserving scalar behavior and the Variable/NamedArray alternative; done means the supported input pattern and resulting documentation are consistent.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- numpy, python
- Domain
- api, data
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100