issue_comments

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.

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.

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

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 https://github.com/pydata/xarray/issues/3891#issuecomment-773693380 and allow passing keep_attrs to xr.where.

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.

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

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.