pydata / xarray

N-D labeled arrays and datasets in Python
https://xarray.dev
Apache License 2.0
3.62k stars 1.08k forks source link

Datatree: parent of new nodes is not assigned properly in update #9345

Closed flamingbear closed 4 weeks ago

flamingbear commented 2 months ago

What is your issue?

Originally posted by @keewis in https://github.com/xarray-contrib/datatree/issues/203

there's something weird happening when assigning nodes using update:

In [1]: import datatree
   ...: import xarray as xr
   ...: 
   ...: tree1 = datatree.DataTree.from_dict({"/a/b": xr.Dataset()})
   ...: node = datatree.DataTree()
   ...: 
   ...: tree2 = tree1.copy()
   ...: tree2.update({"c": node})
   ...: 
   ...: for node in tree3.subtree:
   ...:     print(node.path, node.name, id(node.root))
/ None 140512169735136
/a a 140512169735136
/a/b b 140512169735136
/ c 140512169735248

(the parent of node is None, so its root is the node itself)

So it seems with update / assign you can end up with a inconsistent tree? With https://github.com/xarray-contrib/datatree/pull/201, that inconsistent tree fails to copy because relative_to raises an error.

TomNicholas commented 2 months ago

Is this still relevant?

keewis commented 4 weeks ago

I don't think it is, at least I can't reproduce the original issue with xarray.core.datatree.DataTree.

However, tree.update({"a/b": node}) still does not work (it raises ValueError: node names cannot contain forward slashes), which may or may not be by design.

TomNicholas commented 4 weeks ago

Okay thanks.

Not accepting node names with slashes seems reasonable to me, at least for now. If we generalize that later we can discuss it in a new issue / PR, so I think this one can be closed?