Skip to content

Commit

Permalink
Improved exception types xarray-contrib/datatree#169
Browse files Browse the repository at this point in the history
* created more specific tree-related exception types

* listed new exception types in public API
  • Loading branch information
TomNicholas authored Dec 28, 2022
1 parent 1e4ee72 commit 3761e3a
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 18 deletions.
3 changes: 3 additions & 0 deletions datatree/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from .extensions import register_datatree_accessor
from .io import open_datatree
from .mapping import TreeIsomorphismError, map_over_subtree
from .treenode import InvalidTreeError, NotFoundInTreeError

try:
# NOTE: the `_version.py` file must not be present in the git repository
Expand All @@ -16,6 +17,8 @@
"DataTree",
"open_datatree",
"TreeIsomorphismError",
"InvalidTreeError",
"NotFoundInTreeError",
"map_over_subtree",
"register_datatree_accessor",
"__version__",
Expand Down
6 changes: 4 additions & 2 deletions datatree/tests/test_datatree.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from xarray.tests import create_test_data, source_ndarray

import datatree.testing as dtt
from datatree import DataTree
from datatree import DataTree, NotFoundInTreeError


class TestTreeCreation:
Expand Down Expand Up @@ -109,7 +109,9 @@ def test_relative_paths(self):
assert sue.relative_to(sue) == "."

evil_kate = DataTree()
with pytest.raises(ValueError, match="nodes do not lie within the same tree"):
with pytest.raises(
NotFoundInTreeError, match="nodes do not lie within the same tree"
):
sue.relative_to(evil_kate)


Expand Down
12 changes: 6 additions & 6 deletions datatree/tests/test_treenode.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import pytest

from datatree.iterators import LevelOrderIter, PreOrderIter
from datatree.treenode import NamedNode, TreeError, TreeNode
from datatree.treenode import InvalidTreeError, NamedNode, TreeNode


class TestFamilyTree:
Expand All @@ -21,21 +21,21 @@ def test_parenting(self):
def test_no_time_traveller_loops(self):
john = TreeNode()

with pytest.raises(TreeError, match="cannot be a parent of itself"):
with pytest.raises(InvalidTreeError, match="cannot be a parent of itself"):
john._set_parent(john, "John")

with pytest.raises(TreeError, match="cannot be a parent of itself"):
with pytest.raises(InvalidTreeError, match="cannot be a parent of itself"):
john.children = {"John": john}

mary = TreeNode()
rose = TreeNode()
mary._set_parent(john, "Mary")
rose._set_parent(mary, "Rose")

with pytest.raises(TreeError, match="is already a descendant"):
with pytest.raises(InvalidTreeError, match="is already a descendant"):
john._set_parent(rose, "John")

with pytest.raises(TreeError, match="is already a descendant"):
with pytest.raises(InvalidTreeError, match="is already a descendant"):
rose.children = {"John": john}

def test_parent_swap(self):
Expand Down Expand Up @@ -73,7 +73,7 @@ def test_doppelganger_child(self):
with pytest.raises(TypeError):
john.children = {"Kate": 666}

with pytest.raises(TreeError, match="Cannot add same node"):
with pytest.raises(InvalidTreeError, match="Cannot add same node"):
john.children = {"Kate": kate, "Evil_Kate": kate}

john = TreeNode(children={"Kate": kate})
Expand Down
22 changes: 12 additions & 10 deletions datatree/treenode.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,12 @@
from xarray.core.types import T_DataArray


class TreeError(Exception):
"""Exception type raised when user attempts to create an invalid tree in some way."""
class InvalidTreeError(Exception):
"""Raised when user attempts to create an invalid tree in some way."""

...

class NotFoundInTreeError(ValueError):
"""Raised when operation can't be completed because one node is part of the expected tree."""


class NodePath(PurePosixPath):
Expand Down Expand Up @@ -109,12 +111,12 @@ def _check_loop(self, new_parent: Tree | None) -> None:
"""Checks that assignment of this new parent will not create a cycle."""
if new_parent is not None:
if new_parent is self:
raise TreeError(
raise InvalidTreeError(
f"Cannot set parent, as node {self} cannot be a parent of itself."
)

if self._is_descendant_of(new_parent):
raise TreeError(
raise InvalidTreeError(
"Cannot set parent, as intended parent is already a descendant of this node."
)

Expand Down Expand Up @@ -211,7 +213,7 @@ def _check_children(children: Mapping[str, Tree]) -> None:
if childid not in seen:
seen.add(childid)
else:
raise TreeError(
raise InvalidTreeError(
f"Cannot add same node {name} multiple times as different children."
)

Expand Down Expand Up @@ -524,7 +526,7 @@ def relative_to(self: NamedNode, other: NamedNode) -> str:
If other is not in this tree, or it's otherwise impossible, raise a ValueError.
"""
if not self.same_tree(other):
raise ValueError(
raise NotFoundInTreeError(
"Cannot find relative path because nodes do not lie within the same tree"
)

Expand All @@ -551,7 +553,7 @@ def find_common_ancestor(self, other: NamedNode) -> NamedNode:
break

if not common_ancestor:
raise ValueError(
raise NotFoundInTreeError(
"Cannot find relative path because nodes do not lie within the same tree"
)

Expand All @@ -561,11 +563,11 @@ def _path_to_ancestor(self, ancestor: NamedNode) -> NodePath:
"""Return the relative path from this node to the given ancestor node"""

if not self.same_tree(ancestor):
raise ValueError(
raise NotFoundInTreeError(
"Cannot find relative path to ancestor because nodes do not lie within the same tree"
)
if ancestor.path not in list(a.path for a in self.ancestors):
raise ValueError(
raise NotFoundInTreeError(
"Cannot find relative path to ancestor because given node is not an ancestor of this node"
)

Expand Down
2 changes: 2 additions & 0 deletions docs/source/api.rst
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,8 @@ Exceptions raised when manipulating trees.
:toctree: generated/

TreeIsomorphismError
InvalidTreeError
NotFoundInTreeError

Advanced API
============
Expand Down
1 change: 1 addition & 0 deletions docs/source/whats-new.rst
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ New Features
By `Tom Nicholas <https://github.com/TomNicholas>`_.
- Allow method chaining with a new :py:meth:`DataTree.pipe` method (:issue:`151`, :pull:`156`).
By `Justus Magin <https://github.com/keewis>`_.
- New, more specific exception types for tree-related errors (:pull:`169`).

Breaking changes
~~~~~~~~~~~~~~~~
Expand Down

0 comments on commit 3761e3a

Please sign in to comment.