DataArray.rename does not copy data

[zerothi]

What is your issue?

From the documentation: https://docs.xarray.dev/en/stable/generated/xarray.DataArray.rename.html#xarray.DataArray.rename it isn't clear if the contained data is copied (new memory), or whether it is retained.

It says:

Returns a new DataArray with renamed coordinates, dimensions or a new name.

which may indicate that it copies, however it doesn't:

import numpy as np
import xarray as xr

N = 100
O = xr.DataArray(np.random.rand(N, N), dims=["state [r]", "state [c]"])
s = xr.DataArray(np.random.rand(N, 2), dims=["state [c]", "spin"])

import time
t0 = time.time()
_ = O.rename({"state [r]": "state [c]", "state [c]": "state [r]"})
print("rename: ", time.time() - t0)
print(np.shares_memory(O.values, _.values))

t0 = time.time()
_ = O.values.copy()
print("copy: ", time.time() - t0)
print(np.shares_memory(O.values, _))

Now:

1. I think the current behaviour is correct. It is nice that we can rename stuff, without having to copy the entire array.
2. I only think a clarification is necessary for the documentation.
3. If there are plans for this to copy it, then I think there should atleast be some inplace arguments allowing this.

[max-sixty]

What would you suggest as a change?

[zerothi]

Good question, it probably involves much more elaboration.

Returns a new DataArray with renamed coordinates, dimensions or a new name.
The new DataArray will share memory location with the old DataArray: np.shares_memory(new.values, old.values) is True.

I don't know how to do this succinctly without ambiguity. ;)
It was just a puzzle to me since I didn't know what to expect from doing the operation. And since there is no declaration of the intended use, I am not sure I can trust xarray to do the same in newer releases. Since I would depend on it not making a copy, I just wanted to know what xarray intends to do, and if it wants to keep the current behavior, I would like it emphasized.

I guess the same question/clarification could be said about https://docs.xarray.dev/en/latest/generated/xarray.Dataset.rename.html and friends.

I can make a PR, in case there is a common ground as to what it should say... :)

[max-sixty]

I think the distinction here is between a new DataArray and a copy of the underlying data.

To me that's less confusing but possibly that's because I'm too deep in it.

One issue is that a substantial number of methods do this, and we don't want to have the same description on every method...

[zerothi]

I see, if I read new DataArray I would implicitly assume there is no connection between the new and the old object. Where does the boundary of new lie? Dimension names? Coords and values, data values, attributes, etc? It isn't clear (at least to me) what new encompasses.

I agree about the number of methods that does this would mean a lot of changes, and digging in the code. I could, of course, also be wrong here. But I would think that most users don't know about this detail.

[zerothi]

I.e. this might suprise some users:

import xarray as xr
import numpy as np

data = np.random.rand(3, 3)
print(data)
a = xr.DataArray(
    data,
    dims=list("xy"),
)
print(a)

b = a.rename({"x": "z"})
print(b)

b[0, 1] = 2
print(a)

[max-sixty]

One change could just be:

- Returns a new DataArray with renamed coordinates, dimensions or a new name.
+ Returns a DataArray with renamed coordinates, dimensions or a new name.

...which is possibly less confusing while being simpler. Then if folks want to know about when xarray copies the underlying data, they can look at .copy and learn more there.

Would accept this as a PR unless there are any objections

[zerothi]

I don't see the connection to copy, if anything it should be:

- Returns a new DataArray with renamed coordinates, dimensions or a new name.
+ Returns a  shallow copied(?) DataArray with renamed coordinates, dimensions or a new name.

Is copied necessary here?
for absolute clarity. With your proposal you still don't know if it is a shallow or deep copy. And reading copy does not give any more information.

[max-sixty]

OK, I had thought that this might shed some light on the difference between shallow & deep copies:

xarray/xarray/core/dataarray.py

Lines 1230 to 1237 in a74a605

	"""Returns a copy of this array.
	If `deep=True`, a deep copy is made of the data array.
	Otherwise, a shallow copy is made, and the returned data array's
	values are a new view of this data array's values.
	Use `data` to create a new object with the same structure as
	original but entirely new data.

. On reflection I agree it's a bit tangential.

Personally I think that "Returns a shallow copied DataArray" is too many words + not a great description of what's going on — it's a new object rather than a shallow copy. But if others believe it to be better, very happy to take the majority view.

I would support removing the new given it reduces the word count + I think removes some minor confusion...

And if we don't already have a section on how most method work — i.e. they return a new object with the original data as a property — then that would be a great contribution.

[zerothi]

I agree it isn't easy. Hard to clarify without lots of words. Perhaps for the sake of changing something we could stick with removing new, then if this pops up later, we at least have a reference here.

I just think it would be simpler if the documentation clarifies the intention and behaviour, since currently I do tests to see whats happening, but I don't know if these things changes down the line.

The same thing happened to scipy objects (sparse matrices) where __i<op>__ methods didn't actually behave like one would think. I know it is hard, but clarity is nice! ;)

[max-sixty]

Yes totally!

For xarray, the mental model is that all methods:

• make a new object
• avoid copying the underlying data if possible

I think that mental model gets us to a simple correct answer 98% of the time (the only remaining ambiguities are a couple of small cases on whether a copy is required)

[keewis]

I think that mental model gets us to a simple correct answer 98% of the time (the only remaining ambiguities are a couple of small cases on whether a copy is required)

I think of xarray the same way: everything is shallow copied, with a few exceptions. So going through all the methods and explicitly mention that we shallow-copy feels a bit redundant, but we definitely need to make sure to clearly document the few cases where we deep-copy.

I'm not sure where the best place to set this expectation is, though.

[zerothi]

Perhaps in:

https://docs.xarray.dev/en/stable/getting-started-guide/why-xarray.html
and
https://docs.xarray.dev/en/stable/user-guide/terminology.html

It might be good to emphasize this path?

[max-sixty]

Or given nowhere seems to be a great spot, maybe a question in the https://docs.xarray.dev/en/stable/getting-started-guide/faq.html ?

@zerothi would be a nice small contribution if you're up for it!