[aulemahal]

Pull Request Checklist:

• This PR addresses an already opened issue (for bug fixes / features)
  • This PR fixes Datatree compatability #2127
• Tests for the changes have been added (for bug fixes / features)
  • (If applicable) Documentation has been added / updated (for bug fixes / features)
• CHANGELOG.rst has been updated (with summary of main changes)
  • Link to issue (:issue:number) and pull request (:pull:number) has been added

What kind of change does this PR introduce?

Support xarray.Datatree objects in the ds argument of indicators. When receiving such an object, the indicator computation is mapped over the datasets of the tree.

Passing a Datatree implies setting the as_dataset option to True.

Does this PR introduce a breaking change?

No, it shouldn't. xr.Datatree was added recently, and xclim will not fail if it absent. However, I can't promise it will work with the old standalone xarray-datatree module!

Other information:

This is missing some tests, but most importantly I have a dilemma here : what do we do when a node doesn't have the appropriate variables ?

A Datatree can have nodes with no data, I think it is quite common, depending on your structure. Currently, the code will skip any node that has no "data vars", returning the node untouched. However, any other node that doesn't have the required variables will trigger a failure with a MissingVariableError.

Alternatives would be to return an empty node or return the node untouched.

What does @kobebryant432 think of this question ?

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[kobebryant432]

Hey @aulemahal,

Thanks for tackling this so fast! Really appreciated.

• Skipping empty nodes seems logical, this also maintains the tree structure.
• For nodes that don't have the required variables, I would prefer being explicit. Triggering a failure with a MissingVariableError will inform the user that there is missing data (which may be unexpected).

It may be nice to illustrate how one could than filter the DataTree to include only those leaves (nodes with datasets) that have the required dataset.

[kobebryant432]

Something like this would do the job and allow applying the datatree where possible.

var = "air"
DT.filter(lambda ds: var in ds.data_vars)

[kobebryant432]

Note that currently, this PR does not address convert_units_to functionality.

[kobebryant432]

Started an attempt in #2152

[aulemahal]

@kobebryant432 thanks for your PR. However, after discussions with colleagues, I think I prefer the proposition I just pushed. I made convert_units_to accept Datasets and DataTree natively instead of creating new functions. Does this answer your need ?

It has the same constraint as the indicator : all nodes must have all required variables. However, I did not implement the empty-node skipping in convert_units_to. My reasoning was that I didn't want xclim to be the one handling this and deciding what is an "empty" node. Calling convert_units_to with a Dataset is now easy, so users can use DataTree.map_over_datasets themselves, adding code to skip empty nodes if they so wish. Also, pydata/xarray#10042 will fix the issue by skipping empty nodes by default.

[aulemahal]

@huard I had forgotten, but the decorator was not implemented in xarray (but was in the initial experimental package), so my implementation here is a bit more explicit, but it's only 2 lines of code.

@Zeitsperre 2 more ambiguous input types 🔥

[coveralls]

coverage: 92.338% (-0.1%) from 92.441%
when pulling 78dd222 on datatree-support
into 7027d9b on main.