issue_comments

right, I would leave these methods to xarray extension libraries like cupy-xarray or pint-xarray. I assume you meant .as_pint because adding .to_pint does not make much sense: following the naming convention from above, .to_* should return the duck array, which would make it an alias of either .data or the duck array's constructor called on .data.

Also I can add a .to_pint() method, but I don't really understand what the purpose would be? To be any use to users it would need to accept a units argument, but then it's basically just a less powerful version of pint_xarray's .quantify() accessor method. Can you see any reason to add it @keewis ?

I'm trying to implement this (I wanted pint-aware plotting to work) in #5568, but not 100% sure if I'm doing it right.

~~1) Do we want these as_*, to_* methods to live in Variable or DataArray?~~

2) What about multiply-wrapped arrays? At the moment I have essentially

python def as_numpy(self) -> T_DataArray: """ Coerces wrapped data into a numpy array, and returns it wrapped in a DataArray. """ data = self.data if isinstance(data, dask_array_type): return self.load() elif isinstance(data, cupy_array_type): return self.copy(data=data.get()) elif isinstance(data, pint_array_type): return self.copy(data=data.magnitude) elif isinstance(data, sparse_array_type): return self.copy(data=data.to_dense()) else: return self.copy(data=np.array(data)) but do I actually want multiple passes, like ```python data = self.data if isinstance(data, dask_array_type): data = self.load().data if isinstance(data, cupy_array_type): data = data.get() if isinstance(data, pint_array_type): data = data.magnitude if isinstance(data, sparse_array_type): data = data.to_dense()

return self.copy(data=np.array(data))

```

3) @jacobtomlinson currently .values will extract the values of a cupy array by special-casing it, but based on the above discussion do we actually want to break that in favour of getting them via .to_numpy() instead?

4) There are _as_sparse and _to_dense methods already, but they are private and only present on Variable. Should I expose them?

I agree, this would be useful. Ideally, that would be implemented by deferring to the data, for example by calling the data's to_numpy method (so duck array libraries or duck array integration libraries can customize that), falling back to np.asarray(data).

My impression was that the as_* methods would return xarray objects. So we could have DataArray methods

``` python def as_numpy(self): # needs cupy special handling data = self.data if isinstance(data, cupy_array_type): raise NotImplementedError else: return self.copy(data=np.array(data))

def to_numpy(self): """Coerces to and returns a numpy.ndarray""" return self.as_numpy().data ```

Agreed! Sparse also needs a special case.

On Wed, Jul 22, 2020 at 6:35 PM Deepak Cherian notifications@github.com wrote:

IMO a good first pass for as_numpy would be to use np.asarray with a special case for cupy where it would use .get.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/pydata/xarray/issues/3245#issuecomment-662778974, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJJFVW33SB5YZJWAMIFMR3R46HVVANCNFSM4IOZW5JA .

IMO a good first pass for as_numpy would be to use np.asarray with a special case for cupy where it would use .get.

3. for now we are OK with special as_sparse, as_cupy, as_pint methods in xarray as long as all that logic is contained in one place: as_duck_array.py?

+1 for as_numpy(), too

One question about as_numpy: should it convert pint arrays into NumPy by stripping units? Or should it convert the arrays underlying pint and keep the units? I guess the first would probably make more sense for DataArray.as_numpy(). The second behavior could be achieved with DataArray.pint.as_numpy().

It seems like general consensus was (please correct me if any of this is wrong):

1. .values will return np.asarray(obj.data) and respect the underlying library's choices: this will raise an error with cupy and that's OK.
2. .plot() is a special case and should always coerce to numpy before passing data to matplotlib. plot currently uses .values so we should add a new function that always returns xarray objects with numpy arrays (with cupy as special case) and use that in plot.
3. for now we are OK with special as_sparse, as_cupy, as_pint methods in xarray as long as all that logic is contained in one place: as_duck_array.py?

Sounds good. It might be worth discussing the general strategy for the support of this kind of duck array (sparse, cupy and others) in the dev call on Wednesday.

we currently don't really encourage this way of using accessors (especially using too many of these, see https://github.com/pydata/xarray/issues/1080#issuecomment-258582609 and https://github.com/pydata/xarray/issues/1080#issuecomment-258680571), but you can use register_*_accessor to register functions: ```python @xr.register_dataarray_accessor("as_cupy") def _(da): def as_cupy(self, ...): # ... return da

return as_cupy

```

I am unsure about automatic coercion for .values. I think we've trained users to expect numpy arrays from .values so cupy raising an error is a little unfriendly. OTOH it'd be nice to have something that is equivalent to np.asarray(da.data) but maybe we can expect users who want that to just type it out.

But I think we should automatically coerce for .plot using da.as_numpy().data or something similar. We call .compute() on dask arrays anyway so it's consistent with that behaviour.

What is the best way to save sparse array into a disc?

One naive way would be to use stack -> reset_index, but it flattened coordinates and if there is another variable that depends on these coordinates, they will be also flattened and may consume a lot of space.

assparse could read in at least two different ways, and one of which is not really appropriate.

brilliant

Also isn't the function astype not as_type?

Yes, but I think that's mostly works because it's so short. In general the rule is to use underscores when it improves clarity. assparse could read in at least two different ways, and one of which is not really appropriate.

it makes clear that the objects are still xarray objects

This is a good point. I'm in favour of as_sparse, as_dense.

I like as_sparse and as_dense, because it makes clear that the objects are still xarray objects. I agree with @fujiisoup that to_sparse``todense could be confused to return sparse arrays directly.

I weakly prefer following the upstream API: as_sparse and todense.

though to_sparse,to_dense and to_numpy_data would be more consistent?

Also isn't the function astype not as_type?

Do we arrive at the consensus here for API to change the sparse or numpy array? xref #3542

To make it sparse array, to_sparse() would be better? How about or as_sparse()? + to_sparse() is probably consistent to todense() method + as_sparse() sounds similar to sparse's function, e.g. sparse.as_coo

To change the backend back from sparse array, to_dense() would be better? FYI, sparse uses todense().

I personally like as_sparse or as_numpy (or as_dense?), which sounds similar to as_type, which gives xarray object not dtype itself.

Xarray's .data is basically the equivalent of pandas's .array.

On Fri, Aug 23, 2019 at 2:44 PM Maximilian Roos notifications@github.com wrote:

I do think the pandas approach is pretty good here - don't break anything relying on .values - instead have .array to get the backing array (in whatever form it might be), and to_numpy() coercing to a raw numpy array

ref https://pandas.pydata.org/pandas-docs/stable/whatsnew/v0.24.0.html#accessing-the-values-in-a-series-or-index

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/pydata/xarray/issues/3245?email_source=notifications&email_token=AAJJFVUUGARF55CXIKAQHUDQGBD4NA5CNFSM4IOZW5JKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5BJCVI#issuecomment-524456277, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJJFVWSWKSJFWNUUGRO3ODQGBD4NANCNFSM4IOZW5JA .

I do think the pandas approach is pretty good here - don't break anything relying on .values - instead have .array to get the backing array (in whatever form it might be), and to_numpy() coercing to a raw numpy array

ref https://pandas.pydata.org/pandas-docs/stable/whatsnew/v0.24.0.html#accessing-the-values-in-a-series-or-index

The main downside of to_numpy() is that it isn't obvious whether the result should be a DataArray containing a NumPy array or a raw NumPy array. In particular, pandas uses to_numpy() for converting into a raw NumPy, which is the opposite of our intent here.

:+1: I think to_numpy() would be good.

• Should we formalize the convention that .values always return a numpy array i.e. it will call todense() implicitly and that .data will always return the underlying container: sparse/dask etc.?

I think .values should either return a NumPy array or raise an exception, based upon whether or not the underlying duck array supports coercion with np.array().

We should have a separate API (maybe .to_dense() or .to_numpy_data()?) for explicitly converting into NumPy arrays. This should not be called automatically inside xarray.

Basically, we should leave the decision about whether automatic coercion is safe up to the authors of duck array libraries.

The better way to phrase this is:

1. Should we formalize the convention that .values always return a numpy array i.e. it will call todense() implicitly and that .data will always return the underlying container: sparse/dask etc.?
2. Should we call todense() automatically in plot() the same way we do compute()?

I think it is intentional for automatic coercion to NumPy arrays to fail. Making plot() and .values work should probably require explicitly writing datarray.to_dense() first.