Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better error messages for DataTree #9222

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion xarray/core/datatree.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,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
16 changes: 9 additions & 7 deletions xarray/core/treenode.py
Original file line number Diff line number Diff line change
Expand Up @@ -526,14 +526,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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why you've made this error message less informative @shoyer ? The equivalent operation on a Dataset gives a more explicit message:

xr.Dataset()['a']
KeyError: "No variable named 'a'. Variables on the dataset include []"

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 @@ -581,7 +581,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 @@ -598,7 +598,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 @@ -612,14 +612,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 @@ -630,7 +630,9 @@ def __delitem__(self: Tree, key: str) -> None:
del self._children[key]
child.orphan()
else:
raise KeyError(key)
raise KeyError(key) from ValueError(
"cannot delete key not found in child nodes"
)

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 @@ -204,7 +204,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 @@ -225,7 +225,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 @@ -244,7 +244,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
Loading