issue_comments

The original example how gives an error about dim_1 have different size, but you can still reproduce the behavior if you provide explicit labels for dim_1, e.g., ds = Dataset({"a": DataArray(np.zeros((3, 4)))}, {'dim_1': range(4)}) ds["b"] = b = DataArray(np.zeros((3, 3)), {'dim_1': range(3)}) # !!! we implicitly fill the last column with nans. b[0, 0] = 1 print(ds["b"][0, 0]) # ==> prints 0

The copy argument is still a good idea. We could do: - copy=True always copy - copy=None copy if necessary (default) - copy=False never copy (raise if not possible)

(Note that copy=False in align currently means "copy if necessary", so this would be a breaking change if users are setting that explicitly)

Yes, that's a great idea! We could add a copy=False argument to both the Dataset and DataArray constructors, like the align function.

On Thu, Oct 29, 2015 at 11:58 AM, Joe Hamman notifications@github.com wrote:

Okay, so the Dataset constructor needs to create a copy of the metadata. Could we include a argument to the constructor that copies the .values too? Obviously there is a performance hit there but it would allow users to be explicit about their own memory handling. Just an idea.

Reply to this email directly or view it on GitHub: https://github.com/xray/xray/issues/630#issuecomment-152287198

I think the most confusing part is that this the modifications will trickle down sometimes -- even though I'd prefer them to be always propagated, I would still mind much less if they were never propagated.

I agree! I am going to try to clarify and clean this up a little bit for the next version of xray. The only part that should ever trickle back is the values (because that's unavoidable for high performance). Right now, modifications to the metadata (.attrs) can also sometimes trickle back.

If it is the latter, one can just modify d.values and e.values (as well as their coordinates systems) to point to the new ndarrays, right?

Yes, in principle we could do that. But it's very surprising for a function to modify it's input arguments, so usually we try to avoid things like that.

If you put arrays in a Dataset, you should not count on modifications to the arrays in the Dataset trickling back to the original arrays.

Generally, I would recommend something like the following:

d = DataArray(...) e = DataArray(...) d, e = xray.align(d, e, join='outer') <work to initialize d and e> ds = Dataset({"d": d, "e": e})

or

d = DataArray(...) e = DataArray(...) ds = Dataset({"d": d, "e": e}) <work to initialize ds.d and ds.e>

@anntzer Yes, that would be a reasonable thing to state in the docs. Though I'm not sure how widespread that pattern is -- this is the first time I've seen it used.

"If you use the DataArray constructor instead of inserting an item in a Dataset, the array values will always be a view." Can you explain what you mean here? I am using the DataArray constructor and then inserting the item in the Dataset.

Yes, I should clarify -- the DataArray you create with the constructor will be a view. When you insert it into a Dataset, it may no longer be a view (if the index coordinates are not aligned).

If you use the DataArray constructor instead of inserting an item in a Dataset, the array values will always be a view.

If you pre-align the array you are about to insert using xray.align, it is also guaranteed to be a view.

Unfortunately, it's impossible to make such automatically aligned arrays views in general, because of numpy's memory model.

One thing we could do is add an option to make inserted items always a copy. I would probably put this on the Dataset.merge method, which is what Dataset.__getitem__ uses under the hood.

Yes, this is certainly the case -- do you suggest doing this differently? If coordinates are already aligned, I would rather not do unnecessary copies -- this could be quite expensive.

If this was surprising to you, then it is certainly worth documenting somewhere.