diff --git a/doc/whats-new.rst b/doc/whats-new.rst index 7e3badc7143..ef37ae60421 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -29,6 +29,8 @@ Bug Fixes - Ensure that ``keep_attrs='drop'`` and ``keep_attrs=False`` remove attrs from result, even when there is only one xarray object given to ``apply_ufunc`` (:issue:`10982` :pull:`10997`). By `Julia Signell `_. +- Forbid child names containing ``/`` in ``DataTree`` objects (:issue:`#9978` :issue:`#9490` :pull:`#11014`). + By `Ewan Short `_. Documentation ~~~~~~~~~~~~~ diff --git a/xarray/core/datatree.py b/xarray/core/datatree.py index e079332780c..bd614c914d7 100644 --- a/xarray/core/datatree.py +++ b/xarray/core/datatree.py @@ -223,7 +223,8 @@ def _check_for_slashes_in_names(variables: Iterable[Hashable]) -> None: raise ValueError( "Given variables have names containing the '/' character: " f"{offending_variable_names}. " - "Variables stored in DataTree objects cannot have names containing '/' characters, as this would make path-like access to variables ambiguous." + "Variables stored in DataTree objects cannot have names containing '/' " + "characters, as this would make path-like access to variables ambiguous." ) @@ -532,9 +533,9 @@ def __init__( dataset : Dataset, optional Data to store directly at this node. children : Mapping[str, DataTree], optional - Any child nodes of this node. + Any child nodes of this node. Child names cannot contain the '/' character. name : str, optional - Name for this node of the tree. + Name for this node of the tree. Node name cannot contain the '/' character. Returns ------- diff --git a/xarray/core/treenode.py b/xarray/core/treenode.py index 7eccf09088e..8983db31e67 100644 --- a/xarray/core/treenode.py +++ b/xarray/core/treenode.py @@ -79,7 +79,11 @@ def __init__(self, children: Mapping[str, Self] | None = None): self._children = {} if children: - # shallow copy to avoid modifying arguments in-place (see GH issue #9196) + # shallow copy to avoid modifying arguments in-place (see GH issue #9196), + # first calling _check_children on the constructor input to ensure helpful + # error messages. Note _check_children will be called again on assignment + # to self.children via the setter function, but this is fine. + self._check_children(children) self.children = {name: child.copy() for name, child in children.items()} @property @@ -195,7 +199,7 @@ def children(self) -> None: @staticmethod def _check_children(children: Mapping[str, TreeNode]) -> None: - """Check children for correct types and for any duplicates.""" + """Check children for correct names, types and for any duplicates.""" if not is_dict_like(children): raise TypeError( "children must be a dict-like mapping from names to node objects" @@ -203,10 +207,11 @@ def _check_children(children: Mapping[str, TreeNode]) -> None: seen = set() for name, child in children.items(): + _validate_name(name) if not isinstance(child, TreeNode): raise TypeError( f"Cannot add object {name}. It is of type {type(child)}, " - "but can only add children of type DataTree" + "but can only add children of type DataTree." ) childid = id(child) @@ -666,9 +671,13 @@ def same_tree(self, other: Self) -> bool: def _validate_name(name: str | None) -> None: if name is not None: if not isinstance(name, str): - raise TypeError("node name must be a string or None") + raise TypeError("Node name must be a string or None") if "/" in name: - raise ValueError("node names cannot contain forward slashes") + raise ValueError( + f"Node name '{name}' contains the '/' character. " + "Nodes cannot have names containing '/' characters, as this would make " + "path-like access to nodes ambiguous." + ) class NamedNode(TreeNode): diff --git a/xarray/tests/test_datatree.py b/xarray/tests/test_datatree.py index 0cd888f5782..f53b1485c0c 100644 --- a/xarray/tests/test_datatree.py +++ b/xarray/tests/test_datatree.py @@ -139,6 +139,19 @@ def test_dataset_containing_slashes(self) -> None: ): DataTree(xds) + def test_child_containing_slashes(self) -> None: + ds: Dataset = Dataset({"a": DataArray([1, 2, 3])}) + message = ( + "Node name 'x/y' contains the '/' character. " + "Nodes cannot have names containing '/' characters, as this would make " + "path-like access to nodes ambiguous." + ) + with pytest.raises(ValueError, match=message): + dt: DataTree = DataTree(dataset=ds, children={"x/y": DataTree()}) + with pytest.raises(ValueError, match=message): + dt = DataTree(dataset=ds, children={"x/y": DataTree()}) + dt.children = {"x/y": DataTree()} + class TestPaths: def test_path_property(self) -> None: