keep attrs in xarray.where

[keewis]

Since that question came up at least twice, this allows changing the behavior of where using the keep_attrs option. Dataset.where and DataArray.where always use keep_attrs=True (set in ops.where_method), so we could probably copy that.

Edit: we should also document that only the attributes of cond will be preserved.

• Closes xr.where not preserving attributes #4682, closes xarray.where() drops attributes #4141
• Tests added
• Passes isort . && black . && mypy . && flake8
• User visible changes (including notable bug fixes) are documented in whats-new.rst

[max-sixty]

Great! Thanks @keewis . Could add a whatsnew if you want.

[keewis]

After thinking about this some more, I decided that it would be good to have the behavior of where mirror the behavior of DataArray.where, Dataset.where, and Variable.where. Should we want to be able to control this using set_options we should do so for all them at once.

I also added a entry to whats-new.rst, but I'm not sure about putting this into Breaking changes. Is Bug fixes more appropriate?

Edit: it seems this also preserves the name of cond, which might not be what we want.

[dcherian]

I guess name should be None since we potentially have three possibilities:cond, x, and y?

But that reasoning would also say we should not propagate attrs haha. Not sure what to do.

[keewis]

yes, that's true. Maybe that's why we did not propagate the attrs before, even though it is really easy to implement?

[max-sixty]

I would think the closest match is to return from x — that's the equivalent of x.where(cond, other=y?

[dcherian]

I would think the closest match is to return from x — that's the equivalent of x.where(cond, other=y)?

Great idea!

[keewis]

I would think the closest match is to return from x — that's the equivalent of x.where(cond, other=y?

I'm not sure how to implement that: I can get there by replacing duck_array_ops.where with duck_array_ops.where_method, but that yields a different dimension order after broadcasting (for the doctest examples ["y", "x"] instead of ["x", "y"]). This might not be that much of an issue since we generally tell people to use transpose before applying a function that depends on the order, though.

[mathause]

I can get there by replacing duck_array_ops.where with duck_array_ops.where_method

Is that because apply_ufunc(..., keep_attrs=True) keeps the attrs of the first argument? Could you restore the dimension order after the apply_ufunc or is it not possible to figure it out? Then I'd be fine with a changed order...

[keewis]

Is that because apply_ufunc(..., keep_attrs=True) keeps the attrs of the first argument?

yes, exactly. However, I think it would be easier to extend keep_attrs as described in #3891 (comment) and allow passing keep_attrs to xr.where.

[max-sixty]

Shall we merge?

[keewis]

I'm hoping to wait until combine_attrs is in apply_ufunc, so this is blocked by #5041

[TomNicholas]

@keewis presumbly this could be merged in for the v0.19 release now? #5588

[rjlauer]

Hi, I have a question about this fix relating to coordinate attributes (and am not sure if that deserves a separate issue):
In xarray version 0.17.0, coordinate attributes are being preserved when using xr.where. But, in version 0.19.0, that is not the case anymore. Does the solution for preserving DataArray attributes as discussed here also solve (or rather re-enable) preservation of coordinate attributes? It looks like DataArray.where still preserves coordinate attributes even in version 0.19.0, so I would hope the answer is yes?

[keewis]

yes, that's the plan (we changed the keep_attrs to affect both data / data variables and coordinates in a earlier release, although that's not consistent yet), but I'm surprised to hear that xr.where didn't drop attrs in 0.17.

I'm a bit busy right now, but once things calm down a bit I'll try to finish this.

[Illviljan]

Shall we merge this?
The doctest seems easy to fix. I was just a little surprised that name was considered an attrs as well, but I might not have looked closely enough on that before.

[keewis]

I believe it is best to allow customizing the behavior, so I added a keep_attrs parameter. I named the parameter keep_attrs but it could just as well be called combine_attrs (not sure which is better?) It defaults to dropping the attrs (keep_attrs=False) and will keep the attrs of x for keep_attrs=True. Because we're using apply_ufunc under the hood any value accepted by merge_attrs can be passed, with the only difference being that keep_attrs=True is not equal to keep_attrs="override".

Edit: using a different keep_attrs handler does not affect the name of the resulting object. Because this potentially seems like a breaking change I would do that change in a different PR if we think it is important.

[dcherian]

Thanks @keewis