Add to_numpy() and as_numpy() methods

[TomNicholas]

• Closes sparse and other duck array issues #3245
• Tests added
• Passes pre-commit run --all-files
• User visible changes (including notable bug fixes) are documented in whats-new.rst
• New functions/methods are listed in api.rst

[github-actions]

Unit Test Results

         6 files  ±0           6 suites  ±0   51m 2s ⏱️ ±0s
16 202 tests ±0  14 476 ✔️ ±0  1 726 💤 ±0  0 ❌ ±0 
90 408 runs  ±0  82 242 ✔️ ±0  8 166 💤 ±0  0 ❌ ±0 

Results for commit c5ee050. ± Comparison against base commit c5ee050.

♻️ This comment has been updated with latest results.

[TomNicholas]

Irritating linting error:

xarray/core/variable.py:1095: error: Incompatible return value type (got "Variable", expected "VariableType")  [return-value]
xarray/core/dataarray.py:661: error: Incompatible return value type (got "DataArray", expected "T_DataArray")  [return-value]
Found 2 errors in 2 files (checked 143 source files)

[max-sixty]

@TomNicholas I think that should fix it. But it's become Haskell-esque!

[pep8speaks]

Hello @TomNicholas! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-07-21 21:11:19 UTC

[TomNicholas]

The only failure is the same as in #5600 , so I think this should be ready to merge.

[dcherian]

looks great. Thanks @TomNicholas

[dcherian]

Isn't this always failing? Should we avoid it until such a protocol actually exists?

[shoyer]

That would be my inclination, too.

I can also imagine some other library implementing to_numpy() in a weird way, which could cause issues. At the least, there should be a defensive check of isinstance(data, np.ndarray) afterwards.

[dcherian]

you could stick these in VariableSubclassObjects? Could also do that later if it's a good idea

[TomNicholas]

I just had a go at that and it broke the pint-only test through interaction with TestVariableWithDask, which tries to chunk.

[TomNicholas]

@dcherian I think `.to_numpy()` does actually work in the case of an IndexVariable, because the underlying pandas index has that method. That's not going to be used yet, but we should perhaps leave it in to prepare for the flexible indexes? Other than that I like your other suggestions, please commit and merge as you see fit!

…

On Fri, 16 Jul 2021, 10:44 Deepak Cherian, ***@***.***> wrote: ***@***.**** approved this pull request. looks great. Thanks @TomNicholas <https://github.com/TomNicholas> ------------------------------ In xarray/core/dataarray.py <#5568 (comment)>: > @@ -623,7 +623,7 @@ def __len__(self) -> int: @Property def data(self) -> Any: - """The array's data as a dask or numpy array""" + """The array's data as a numpy-like array""" ⬇️ Suggested change - """The array's data as a numpy-like array""" + """ + The DataArray's data as an array. The underlying array type + (e.g. dask, sparse, pint) is preserved. + + See Also + -------- + DataArray.to_numpy + DataArray.as_numpy + DataArray.values + """ ------------------------------ In xarray/core/dataarray.py <#5568 (comment)>: > @@ -632,13 +632,42 @@ def data(self, value: Any) -> None: @Property def values(self) -> np.ndarray: - """The array's data as a numpy.ndarray""" + """ + The array's data as a numpy.ndarray. + + If the array's data is not a numpy.ndarray this will attempt to convert + it naively using np.array(), which will raise an error if the array + type does not support coercion like this. ⬇️ Suggested change - type does not support coercion like this. + type does not support coercion like this (e.g. cupy). ------------------------------ In xarray/core/dataarray.py <#5568 (comment)>: > return self.variable.values @values.setter def values(self, value: Any) -> None: self.variable.values = value + def to_numpy(self) -> np.ndarray: + """ + Coerces wrapped data to numpy and returns a numpy.ndarray. + + See also + -------- + DataArray.as_numpy : Same but returns the surrounding DataArray instead. + Dataset.as_numpy ⬇️ Suggested change - Dataset.as_numpy + Dataset.as_numpy + DataArray.values + DataArray.data ------------------------------ In xarray/core/dataarray.py <#5568 (comment)>: > + + See also + -------- + DataArray.as_numpy : Same but returns the surrounding DataArray instead. + Dataset.as_numpy + """ + return self.variable.to_numpy() + + def as_numpy(self: T_DataArray) -> T_DataArray: + """ + Coerces wrapped data and coordinates into numpy arrays, returning a DataArray. + + See also + -------- + DataArray.to_numpy : Same but returns only the data as a numpy.ndarray object. + Dataset.as_numpy : Converts all variables in a Dataset. ⬇️ Suggested change - Dataset.as_numpy : Converts all variables in a Dataset. + Dataset.as_numpy : Converts all variables in a Dataset. + DataArray.values + DataArray.data ------------------------------ In xarray/core/pycompat.py <#5568 (comment)>: > + self.module = duck_array_module + self.version = duck_array_version + self.type = duck_array_type + self.available = self.version > "0.0.0" ⬇️ Suggested change - self.available = self.version > "0.0.0" + self.available = duck_array_module is not None v. minor: this seems cleaner? ------------------------------ In xarray/core/variable.py <#5568 (comment)>: > +dask_array_type = DuckArrayModule("dask").type +cupy_array_type = DuckArrayModule("cupy").type +sparse_array_type = DuckArrayModule("sparse").type import these from pycompat as earlier? ------------------------------ In xarray/core/variable.py <#5568 (comment)>: > + try: + data = data.to_numpy() + except AttributeError: Isn't this always failing? Should we avoid it until such a protocol actually exists? ------------------------------ In xarray/core/variable.py <#5568 (comment)>: > @@ -1069,6 +1069,32 @@ def chunk(self, chunks={}, name=None, lock=False): return self._replace(data=data) + def to_numpy(self) -> np.ndarray: + """Coerces wrapped data to numpy and returns a numpy.ndarray""" + # TODO an entrypoint so array libraries can choose coercion method? + data = self.data + try: + data = data.to_numpy() + except AttributeError: + if isinstance(data, dask_array_type): + data = self.compute().data ⬇️ Suggested change - data = self.compute().data + data = data.compute() to match the rest? ------------------------------ In xarray/tests/test_variable.py <#5568 (comment)>: > @@ -2540,3 +2542,68 @@ def test_clip(var): var.mean("z").data[:, :, np.newaxis], ), ) + + ***@***.***("Var", [Variable, IndexVariable]) you could stick these in VariableSubclassObjects? Could also do that later if it's a good idea — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#5568 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AISNPIYHDVQ7DOWCTQH4REDTYBOW5ANCNFSM47XKDGNQ> .

[TomNicholas]

The only error is the ffspec error now

[shoyer]

That would be my inclination, too.

I can also imagine some other library implementing to_numpy() in a weird way, which could cause issues. At the least, there should be a defensive check of isinstance(data, np.ndarray) afterwards.

[TomNicholas]

Okay thanks @shoyer - I've removed the type check and the attempt to call .to_numpy().