Skip to content

Commit

Permalink
Fix name permanence behaviour in update method xarray-contrib/datatre…
Browse files Browse the repository at this point in the history
…e#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>
  • Loading branch information
TomNicholas and pre-commit-ci[bot] authored Jan 4, 2023
1 parent ad86f5e commit 9467ac1
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 2 deletions.
8 changes: 7 additions & 1 deletion datatree/datatree.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions datatree/tests/test_datatree.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
2 changes: 1 addition & 1 deletion docs/source/whats-new.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 9467ac1

Please sign in to comment.