xarray icon indicating copy to clipboard operation
xarray copied to clipboard

Forbid path-like DataTree child names #9490

Open eshort0401 opened this issue 2 months ago • 1 comments

  • [x] Closes #9490
  • [x] Closes #9978
  • [x] Tests added
  • [x] User visible changes (including notable bug fixes) are documented in whats-new.rst

Addresses #9490 and #9978 by forbidding the / character in DataTree child names.

  1. DataTree node names are already checked using _validate_name from the treenode module. I therefore call this function inside the _check_children member function of TreeNode. This check ensures invalid children arguments are caught early in the DataTree constructor, and when the DataTree.children attribute is reset explicitly (see #9441). I have also reworded the _validate_name error message slightly for consistency with _check_for_slashes_in_names (defined in the datatree module) which checks variable names. To illustrate, code like
    import xarray as xr
    ds = xr.Dataset({"a": xr.DataArray([1, 2, 3])})
    dt = xr.DataTree(dataset=ds, children={"x/y": xr.DataTree()})
    
    and
    import xarray as xr
    ds = xr.Dataset({"a": xr.DataArray([1, 2, 3])})
    dt = xr.DataTree(dataset=ds, children={"x": xr.DataTree()})
    dt.children = {"x/y": xr.DataTree()}
    
    will now both produce
    ValueError: Node name 'x/y' contains the '/' character. Nodes cannot have names containing '/' characters, as this would make path-like access to nodes ambiguous.
    
  2. I have also added a call to _check_children on the input children mapping given to the DataTree constructor, before it is assigned and the setter function is called. This additional check ensures that if the user passes a bad children, a helpful error message is given before the shallow copy {name: child.copy() for name, child in children.items()} fails.
  3. I have added a couple of lines to the DataTree.__init__ docstring to communicate that / characters are forbidden in both the name argument, and the keys of the children mapping argument.

Additional Notes

This PR follows after discussion with @TomNicholas in #11001. See also #9441 and #9477.

eshort0401 avatar Dec 14 '25 03:12 eshort0401

I've just realized this PR is very similar to @TomNicholas's PR #9492 from 2024, sorry! Happy to update authorship in the whats-new.rst if appropriate. Not sure why #9492 wasn't merged as a temporary fix for #9485.

eshort0401 avatar Dec 15 '25 00:12 eshort0401