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

The only error is the ffspec error now

@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 https://github.com/pydata/xarray/pull/5568#discussion_r671384844:

@@ -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 https://github.com/pydata/xarray/pull/5568#discussion_r671385021:

@@ -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 https://github.com/pydata/xarray/pull/5568#discussion_r671385936:

     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 https://github.com/pydata/xarray/pull/5568#discussion_r671386127:

+

• 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 https://github.com/pydata/xarray/pull/5568#discussion_r671387856:

• 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 https://github.com/pydata/xarray/pull/5568#discussion_r671388121:

+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 https://github.com/pydata/xarray/pull/5568#discussion_r671388613:

• 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 https://github.com/pydata/xarray/pull/5568#discussion_r671389110:

@@ -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 https://github.com/pydata/xarray/pull/5568#discussion_r671391347:

@@ -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 https://github.com/pydata/xarray/pull/5568#pullrequestreview-708566048, or unsubscribe https://github.com/notifications/unsubscribe-auth/AISNPIYHDVQ7DOWCTQH4REDTYBOW5ANCNFSM47XKDGNQ .

The only failure is the same as in https://github.com/pydata/xarray/issues/5600 , so I think this should be ready to merge.

This is actually ready for a review now @shoyer

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)