Skip to content

Commit

Permalink
better error messages for DataTree
Browse files Browse the repository at this point in the history
  • Loading branch information
shoyer committed Jul 9, 2024
1 parent 179c670 commit 8e1501e
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 11 deletions.
2 changes: 1 addition & 1 deletion xarray/core/datatree.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ def __init__(
coords: Mapping[Any, Any] | None = None,
attrs: Mapping[Any, Any] | None = None,
):
raise AttributeError("DatasetView objects are not to be initialized directly")
raise TypeError("DatasetView objects are not to be initialized directly")

@classmethod
def _constructor(
Expand Down
17 changes: 10 additions & 7 deletions xarray/core/treenode.py
Original file line number Diff line number Diff line change
Expand Up @@ -456,14 +456,14 @@ def _get_item(self: Tree, path: str | NodePath) -> Tree | T_DataArray:
for part in parts:
if part == "..":
if current_node.parent is None:
raise KeyError(f"Could not find node at {path}")
raise KeyError(path)
else:
current_node = current_node.parent
elif part in ("", "."):
pass
else:
if current_node.get(part) is None:
raise KeyError(f"Could not find node at {path}")
raise KeyError(path)
else:
current_node = current_node.get(part)
return current_node
Expand Down Expand Up @@ -511,7 +511,7 @@ def _set_item(
path = NodePath(path)

if not path.name:
raise ValueError("Can't set an item under a path which has no name")
raise ValueError("cannot set an item under a path which has no name")

if path.root:
# absolute path
Expand All @@ -528,7 +528,7 @@ def _set_item(
if part == "..":
if current_node.parent is None:
# We can't create a parent if `new_nodes_along_path=True` as we wouldn't know what to name it
raise KeyError(f"Could not reach node at path {path}")
raise ValueError(f"could not reach node at path {path}")
else:
current_node = current_node.parent
elif part in ("", "."):
Expand All @@ -542,14 +542,14 @@ def _set_item(
current_node._set(part, new_node)
current_node = current_node.children[part]
else:
raise KeyError(f"Could not reach node at path {path}")
raise ValueError(f"could not reach node at path {path}")

if name in current_node.children:
# Deal with anything already existing at this location
if allow_overwrite:
current_node._set(name, item)
else:
raise KeyError(f"Already a node object at path {path}")
raise ValueError(f"already a node object at path {path}")
else:
current_node._set(name, item)

Expand All @@ -560,7 +560,10 @@ def __delitem__(self: Tree, key: str):
del self._children[key]
child.orphan()
else:
raise KeyError("Cannot delete")
try:
raise ValueError("cannot delete key not found in child nodes")
except ValueError as e:
raise KeyError(key) from e

def same_tree(self, other: Tree) -> bool:
"""True if other node is in the same tree as this node."""
Expand Down
6 changes: 3 additions & 3 deletions xarray/tests/test_treenode.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ def test_child_already_exists(self):
mary: TreeNode = TreeNode()
john: TreeNode = TreeNode(children={"Mary": mary})
mary_2: TreeNode = TreeNode()
with pytest.raises(KeyError):
with pytest.raises(ValueError):
john._set_item("Mary", mary_2, allow_overwrite=False)

def test_set_grandchild(self):
Expand All @@ -184,7 +184,7 @@ def test_create_intermediate_child(self):
rose: TreeNode = TreeNode()

# test intermediate children not allowed
with pytest.raises(KeyError, match="Could not reach"):
with pytest.raises(ValueError, match="could not reach"):
john._set_item(path="Mary/Rose", item=rose, new_nodes_along_path=False)

# test intermediate children allowed
Expand All @@ -203,7 +203,7 @@ def test_overwrite_child(self):

# test overwriting not allowed
marys_evil_twin: TreeNode = TreeNode()
with pytest.raises(KeyError, match="Already a node object"):
with pytest.raises(ValueError, match="already a node object"):
john._set_item("Mary", marys_evil_twin, allow_overwrite=False)
assert john.children["Mary"] is mary
assert marys_evil_twin.parent is None
Expand Down

0 comments on commit 8e1501e

Please sign in to comment.