In order to maintain a list of currently relevant issues, we mark issues as stale after a period of inactivity

If this issue remains relevant, please comment here or remove the stale label; otherwise it will be marked as closed automatically

In order to maintain a list of currently relevant issues, we mark issues as stale after a period of inactivity

If this issue remains relevant, please comment here or remove the stale label; otherwise it will be marked as closed automatically

Looking at @crusaderky's example of different coordinate labels again, I finally remember why it works this way.

The logic of ds.update(other) is that (1) variables explicitly listed in other should take precedence over the original object and (2) mutating a Dataset should not change its dimensions or indexes.

This is pretty clearly expressed in the original code: return merge_core([dataset, other], priority_arg=1, indexes=dataset.indexes)

In @crusaderky's example with fridge.update(shopping), shopping first gets reindexed to fridge (which means it ends up only holding NaN), and is then used to override the original dataset: <xarray.Dataset> Dimensions: (fruit: 1) Coordinates: * fruit (fruit) object 'apples' quality (fruit) object nan Data variables: fruits (fruit) float64 nan It would probably make sense to keep values from the original variables rather than blindly replacing them with the new NaNs from shopping, but in general I do think the general approach of "right join on variables" and "left join on indexes" makes sense for update().

For most use cases, the true outer join makes more sense -- which is why xarray.merge() works that way.

For referene the original issue in #2068 was ```python In [4]: ds = xr.Dataset() ...: ds.coords['source'] = (['a', 'b', 'c'], np.random.random((2, 3, 4))) ...: ds.coords['unrelated'] = (['a', 'c'], np.random.random((2, 4))) ...: ds ...: Out[4]: <xarray.Dataset> Dimensions: (a: 2, b: 3, c: 4) Coordinates: source (a, b, c) float64 0.4158 0.07152 0.4258 0.4382 0.6616 0.142 ... unrelated (a, c) float64 0.9318 0.03723 0.4226 0.9472 0.8753 0.7022 ... Dimensions without coordinates: a, b, c Data variables: empty

In [5]: ds['dest-2'] = xr.ones_like(ds['source'].isel(c=0)) ...: ds ...: Out[5]: <xarray.Dataset> Dimensions: (a: 2, b: 3) Coordinates: source (a, b) float64 0.4158 0.6616 0.1583 0.7821 0.221 0.2555 unrelated (a) float64 0.9318 0.8753 Dimensions without coordinates: a, b Data variables: dest-2 (a, b) float64 1.0 1.0 1.0 1.0 1.0 1.0 `` whereds['unrelated']drops dimensionc`. We changed this behavior in #2087, but I think it was a wrong direction.

The previous behavior might be OK as long as unrelated is a coordinate variable.

EDIT: I still feel something strange both in the previous and current behavior of __setitem__ with coord. Generally, as @crusaderky has pointed out, the right join will be a better choice. But In the above example, dropping the dimension of c of 'unrelated' looks also awkward.

Thanks, @crusaderky. The first behavior you pointed out is a bug I think. I raised an issue in #2184, and maybe it should be discussed there.

For the second example,

I think this should be a right join.

I agree with this.

If there are conflicts in dimension coordinate, should it be outer join?

Consider this example: a = Dataset({ 'x': [10, 20], 'd1': ('x', [100, 200]), 'd2': ('x', [300, 400]) }) b = Dataset({ 'x': [15], 'd1': ('x', [500]), }) a.update(b) In the above, with anything but an outer join you're destroying d2 - which doesn't even exist in the rhs dataset! A sane, desirable outcome should be Dataset({ 'x': [10, 20, 15], 'd1': ('x', [nan, nan, 500]), 'd2': ('x', [300, 400, nan]) })

If there are no conflicts in dimension coordinate, but there are conflicts in non dimension coordinate, whether left or right should be prioritized?

I think this should be a right join. I alway think of non-index coords as N-to-1 properties of the index. For example, a = Dataset( coords={ 'country': ('country', ['UK', 'France', 'Greece']), 'currency': ('country', ['GBP', 'EUR', 'EUR']), }, data_vars={ 'GDP': ('country', [1000, 2000, 3000]), 'Debt': ('country', [100, 200, 300]), }) b = Dataset( # Greece exits the Eurozone coords={ 'country': ('country', ['UK', 'France', 'Greece']), 'currency': ('country', ['GBP', 'EUR', 'GRD']), }, data_vars={ 'GDP': ('country', [1000, 2000, 150000]), }) a.update(b) In the above example, I just broke the Debt variable - as I forgot to perform a currency conversion for the greek debt, which has been silently changed from 300 EUR to 300 GRD. However I can't see any elegant way to avoid this. I definitely would not like to duplicate the 'country' index.

So maybe we can leave the current behavior as is for now (but remove the warning).

Agreed.

~@shoyer, how do you think about the current __setitem__ behavior with conflict dimension coordinate? Should it be outer join as @crusaderky pointed out?~

EDIT: I did not noticed the above comment. I will raise an issue for this.

@crusaderky this behavior you show is indeed really strange. I don't know why alignment of dimensions works that way currently.

OK, looking at this more carefully ds.update(other) didn't actually change when other is a Dataset, because ds[k] = ds[k].drop(coord_names) doesn't actually drop coordinates from a Dataset. It just shows a warning now, due to iteration over a Dataset.

So maybe we can leave the current behavior as is for now (but remove the warning).

What did change is how we handle conflicts in __setitem__ (which was intentional), and how we handle conflicts in update when the new value is a dictionary (which was not intentional, but at least remained consistent with __setitem__).

2087 changed the second behavior.

```python In [1]: import xarray ...: ...: fridge = xarray.Dataset( ...: data_vars={ ...: 'var1': ('fruit', [10]), ...: }, ...: coords={ ...: 'fruit': ('fruit', [1]), ...: 'quality': ('fruit', ['Red Velvet']), ...: }) ...: shopping = xarray.Dataset( ...: data_vars={ ...: 'var1': ('fruit', [20]), ...: }, ...: coords={ ...: 'fruit': ('fruit', [1]), ...: 'quality': ('fruit', ['Tangerine']), ...: }) ...: ...: fridge['var1'] = shopping['var1'] ...:

```

with v10.3 python In [2]: fridge Out[2]: <xarray.Dataset> Dimensions: (fruit: 1) Coordinates: * fruit (fruit) int64 1 quality (fruit) <U9 'Tangerine' Data variables: var1 (fruit) int64 20 and with v10.4 python In [3]: fridge Out[3]: <xarray.Dataset> Dimensions: (fruit: 1) Coordinates: * fruit (fruit) int64 1 quality (fruit) <U10 'Red Velvet' Data variables: var1 (fruit) int64 20

I got cofused... I was wrong. I thought it would be an outer joing

I think we need to discuss two issues here separately. 1. If there are conflicts in dimension coordinate, should it be outer join? 2. If there are no conflicts in dimension coordinate, but there are conflicts in non dimension coordinate, whether left or right should be prioritized?

Then I'm not sure I understand the change: fridge = xarray.Dataset( data_vars={ 'fruits': ('fruit', [10]), }, coords={ 'fruit': ('fruit', ['apples']), 'quality': ('fruit', ['Red Velvet']), }) shopping = xarray.Dataset( data_vars={ 'fruits': ('fruit', [20]), }, coords={ 'fruit': ('fruit', ['oranges']), 'quality': ('fruit', ['Tangerine']), }) fridge.update(shopping) # throw away old contents and fill with new stuff In both 0.10.3 and 0.10.4: <xarray.Dataset> Dimensions: (fruit: 1) Coordinates: * fruit (fruit) object 'apples' quality (fruit) object nan Data variables: fruits (fruit) float64 nan The above doesn't make any sense to me. I wanted to replace the fruits variable with brand new content, and instead I lost both the old and the new?!?

I think we should discuss dimension coordinte and non-dimenson coordinate separately. I guess @shoyer meant non-dimenson coordinate here.

For dimension coordinte, it is always outer join, if I understand correctly.

I'm of the strong opinion that all joins should be outer joins unless the user explicitly says otherwise, as it's the approach least prone to do damage. I would humbly suggest considering the change for a future major release (0.11 / 0.12), with several minor releases before that printing futurewarnings.

This said, I think that changing from a right join (0.10.3) to a left join (0.10.4) will only cause breakages without providing any actual benefit in terms of user-friendliness, so we should retain the previous behaviour. A right join vaguely makes more sense IMHO as it follows the general phylosophy of dict.update() where rhs wins in case of collision.

I think dataset.update(other) should be equivalent with python for key, value in other.items(): dataset[key] = value as similar to python native dict.

Our .item() ony iterates over data_vars not coordinate. So I think even in dataset.update(other) coordinates from other should be dropped if there is a conflict.

cc @fujiisoup @crusaderky