issue_comments

I'll add one in one of the follow-up PRs

:+1:. I tried modifying fillna, but that turned out to be harder than I expected.

As a summary, per-variable fill values for fillna and where are still left for other PRs.

does anyone have any advice on how to add multiple fill value support to fillna? It is currently using ops.fillna which uses apply_ufunc with duck_array_ops.fillna: https://github.com/pydata/xarray/blob/d9ebcafaddf0bb80052d30f43ca5ea18a5c7229f/xarray/core/ops.py#L137-L171 but that obviously only works for data variables.

To fix that, I would manually apply duck_array_ops.fillna to data variables and coordinates and then reassemble the dataset (using _to_temp_dataset / _from_temp_dataset for DataArray), but since I don't really understand apply_ufunc I'm not sure what that would break.

Here's the list of functions I'm considering to extend: * [x] concat / merge / align / combine_* (but aligning DataArray objects and combine_by_coords have no tests for different fill values) * [ ] apply_ufunc (?) * [x] full_like (for Dataset args) * [x] Dataset.reindex / Dataset.reindex_like * [x] DataArray.reindex / DataArray.reindex_like (for non-dimension coordinates) * [x] Dataset.unstack * [x] DataArray.unstack (works since it delegates to Dataset.unstack, but doesn't have tests) * [x] Dataset.merge * [x] Dataset.shift * [ ] ~DataArray.shift (probably doesn't make sense, though)~ * [ ] Dataset.pad (already works) * [ ] DataArray.pad (already works) * [ ] Dataset.idxmin / Dataset.idxmax: uses Dataset.map and is quite difficult to extend * [ ] ~DataArray.idxmin / DataArray.idxmax~: doesn't make much sense, I think * [ ] Dataset.fillna (already works for data variables, but difficult to extend to coordinates) * [ ] DataArray.fillna * [ ] ~Dataset.rolling.construct (not sure if that makes sense)~ * [ ] ~Dataset.interp / DataArray.interp (for extrapolation, using the kwargs parameter): we probably shouldn't support this, but instead recommend ds.interp(...).fillna({"a": ..., "b": ...}).~

apply_ufunc turns out to be way too complicated for me to change it without understanding most of the code, so I guess we should postpone that until someone needs multiple fill values on dataset objects?

I think fillna should allow filling coordinates (both dimension and non-dimension coordinates), but the current implementation simply uses apply_ufunc with duck_array_ops.fillna, so I'm not sure how to support that.

Same for idxmin and idxmax: they use Dataset.map or call the computation function directly, so we'd need to rewrite those functions to get fill values.

1. me neither, that's why I added the question mark (I'll investigate a bit more)
2. agreed
3. yep, the docs say:

   Only data variables are moved; coordinates stay in place. This is consistent with the behavior of shift in pandas.

and the code doesn't touch the coordinates (not even non-dimension coordinates).

Should we add more tests for higher level functions

I have been focusing on functions that use reindex, but I agree that others like fillna / bfill / ffill and unstack should allow mappings, too.