Forbid path-like DataTree child names #9490

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 <https://github.com/jsignell>`_.
+- Forbid child names containing ``/`` in ``DataTree`` objects (:issue:`#9978` :issue:`#9490` :pull:`#11014`).
+  By `Ewan Short <https://github.com/eshort0401>`_.
 
 Documentation
 ~~~~~~~~~~~~~

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
         -------

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,18 +199,19 @@ 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"
             )
 
         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):

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: