From 8e1501e5e47cf681207f387ad045723d3c740bc6 Mon Sep 17 00:00:00 2001 From: Stephan Hoyer Date: Tue, 9 Jul 2024 10:59:19 -0700 Subject: [PATCH 1/3] better error messages for DataTree --- xarray/core/datatree.py | 2 +- xarray/core/treenode.py | 17 ++++++++++------- xarray/tests/test_treenode.py | 6 +++--- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/xarray/core/datatree.py b/xarray/core/datatree.py index 65ff8667cb7..7e53f79ddbb 100644 --- a/xarray/core/datatree.py +++ b/xarray/core/datatree.py @@ -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( diff --git a/xarray/core/treenode.py b/xarray/core/treenode.py index 77e7ed23a51..391d06a46a2 100644 --- a/xarray/core/treenode.py +++ b/xarray/core/treenode.py @@ -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 @@ -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 @@ -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 ("", "."): @@ -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) @@ -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.""" diff --git a/xarray/tests/test_treenode.py b/xarray/tests/test_treenode.py index a7de2e39af6..d96c2def168 100644 --- a/xarray/tests/test_treenode.py +++ b/xarray/tests/test_treenode.py @@ -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): @@ -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 @@ -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 From 39ebdf9a11775ffa6a14bb5e927128a20b97a47d Mon Sep 17 00:00:00 2001 From: Tom Nicholas Date: Tue, 20 Aug 2024 09:54:26 -0600 Subject: [PATCH 2/3] Improve raising of KeyError from ValueError --- xarray/core/treenode.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/xarray/core/treenode.py b/xarray/core/treenode.py index 391d06a46a2..f9fe8a2e3d1 100644 --- a/xarray/core/treenode.py +++ b/xarray/core/treenode.py @@ -560,10 +560,7 @@ def __delitem__(self: Tree, key: str): del self._children[key] child.orphan() else: - try: - raise ValueError("cannot delete key not found in child nodes") - except ValueError as e: - raise KeyError(key) from e + 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.""" From 7d118d729d1e4a4174fd6fe75143695b0b1714e0 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 20 Aug 2024 15:56:00 +0000 Subject: [PATCH 3/3] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- xarray/core/treenode.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/xarray/core/treenode.py b/xarray/core/treenode.py index f9fe8a2e3d1..c0ece086880 100644 --- a/xarray/core/treenode.py +++ b/xarray/core/treenode.py @@ -560,7 +560,9 @@ def __delitem__(self: Tree, key: str): del self._children[key] child.orphan() else: - raise KeyError(key) from ValueError("cannot delete key not found in child nodes") + 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."""