Fix name permanence behaviour in update method (#178)

* test for name permanence in update

* ensure node is copied on update

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* black

* whatsnew

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

Author: TomNicholas

datatree/datatree.py

@@ -577,6 +577,9 @@ def _replace(
         datatree. It is up to the caller to ensure that they have the right type
         and are not used elsewhere.
         """
+        # TODO Adding new children inplace using this method will cause bugs.
+        # You will end up with an inconsistency between the name of the child node and the key the child is stored under.
+        # Use ._set() instead for now
         if inplace:
             if variables is not None:
                 self._variables = variables
@@ -801,7 +804,10 @@ def update(self, other: Dataset | Mapping[str, DataTree | DataArray]) -> None:
         new_variables = {}
         for k, v in other.items():
             if isinstance(v, DataTree):
-                new_children[k] = v
+                # avoid named node being stored under inconsistent key
+                new_child = v.copy()
+                new_child.name = k
+                new_children[k] = new_child
             elif isinstance(v, (DataArray, Variable)):
                 # TODO this should also accommodate other types that can be coerced into Variables
                 new_variables[k] = v

datatree/tests/test_datatree.py

@@ -213,6 +213,13 @@ def test_update_new_named_dataarray(self):
         expected = da.rename("results")
         xrt.assert_equal(folder1["results"], expected)
 
+    def test_update_doesnt_alter_child_name(self):
+        dt = DataTree()
+        dt.update({"foo": xr.DataArray(0), "a": DataTree(name="b")})
+        assert "a" in dt.children
+        child = dt["a"]
+        assert child.name == "a"
+
 
 class TestCopy:
     def test_copy(self, create_test_datatree):

docs/source/whats-new.rst

@@ -37,7 +37,7 @@ Breaking changes
 
 - :py:meth:`DataTree.copy` copy method now only copies the subtree, not the parent nodes (:pull:`171`).
   By `Tom Nicholas <https://github.com/TomNicholas>`_.
-- Grafting a subtree onto another tree now leaves name of original subtree object unchanged (:issue:`116`, :pull:`172`).
+- Grafting a subtree onto another tree now leaves name of original subtree object unchanged (:issue:`116`, :pull:`172`, :pull:`178`).
   By `Tom Nicholas <https://github.com/TomNicholas>`_.
 
 Deprecations