From 5791540fee8aac6cd7352835a63768dd2d9f1c58 Mon Sep 17 00:00:00 2001 From: Verweijen Date: Mon, 21 Jul 2025 23:52:55 +0200 Subject: [PATCH 1/2] Improve route code --- src/abstracttree/route.py | 167 +++++++++++++++++--------------------- tests/test_route.py | 73 ++++++++++++++--- 2 files changed, 134 insertions(+), 106 deletions(-) diff --git a/src/abstracttree/route.py b/src/abstracttree/route.py index 7842b5c..99bc473 100644 --- a/src/abstracttree/route.py +++ b/src/abstracttree/route.py @@ -1,146 +1,127 @@ import itertools -from abc import ABCMeta from bisect import bisect -from collections.abc import Sized, Sequence, MutableSequence -from functools import lru_cache -from typing import TypeVar, Optional +from collections.abc import Sequence, MutableSequence +from typing import TypeVar, Optional, Generic -from . import iterators as _iterators -from .generics import TreeLike, nid +from abstracttree import iterators as _iterators +from abstracttree.generics import TreeLike, nid -TNode = TypeVar("TNode", bound=TreeLike) +T = TypeVar("T", bound=TreeLike) -class Route: +class Route(Generic[T]): """Representation of a route trough adjacent nodes in the tree. Two nodes are adjacent if they have a parent-child relationship. The route will be as short as possible, but it will visit the anchor points in order. """ - __slots__ = "_apaths", "_lca" + __slots__ = "_anchor_paths", "_ancestor_levels" - def __init__(self, *anchors: TreeLike): + def __init__(self, *anchors: T): """Create a route through a few nodes. All nodes should belong to the same tree. """ - self._apaths: MutableSequence[Sequence[TNode]] = [] - self._lca = None + self._anchor_paths: MutableSequence[Sequence[T]] = [] + self._ancestor_levels = [] for anchor in anchors: self.add_anchor(anchor) def __repr__(self): - nodes_str = ", ".join([repr(p[-1]) for p in self._apaths]) + nodes_str = ", ".join([repr(anchor) for anchor in self.anchors]) return f"{self.__class__.__name__}({nodes_str})" - def add_anchor(self, anchor: TreeLike): + def add_anchor(self, anchor: T): """Add a node to the route. The node should belong to the same tree as any existing anchor nodes. """ - self._lca = None + anchor_path = list(_iterators.path(anchor)) - apaths = self._apaths - if not apaths or nid(apaths[0][0]) == nid(anchor_path[0]): - apaths.append(anchor_path) - else: - raise ValueError("Different tree!") + if self._anchor_paths: + last_path = self._anchor_paths[-1] + if anchor_path[0] != last_path[0]: + raise ValueError("Different tree!") + self._ancestor_levels.append(_common2(last_path, anchor_path)) + + self._anchor_paths.append(anchor_path) + + def __iter__(self): + if len(self._anchor_paths) < 2: + yield from self.anchors + path_j = None + for (path_i, path_j), level in zip(itertools.pairwise(self._anchor_paths), self._ancestor_levels): + yield from path_i[:level:-1] + yield from path_j[level:-1] + if path_j is not None: + yield path_j[-1] + + def __reversed__(self): + if len(self._anchor_paths) < 2: + yield from self.anchors + path_j = None + for (path_i, path_j), level in zip(itertools.pairwise(reversed(self._anchor_paths)), + reversed(self._ancestor_levels)): + yield from path_i[:level:-1] + yield from path_j[level:-1] + if path_j is not None: + yield path_j[-1] + + def __len__(self) -> int: + p, l = self._anchor_paths, self._ancestor_levels + if len(p) < 2: + return len(p) + return 1 + len(p[0]) + len(p[-1]) + 2 * (sum(map(len, p[1:-1])) - sum(l) - len(l)) + + count = __len__ @property def anchors(self): """View of the anchor nodes.""" - return AnchorsView(self, self._apaths) + return [path[-1] for path in self._anchor_paths] @property def nodes(self): """View of all nodes that make up the route.""" - return NodesView(self, self._apaths) + return self @property def edges(self): """View of all edges that make up the route.""" - return EdgesView(self, self._apaths) + return EdgesView(self) @property - def lca(self) -> Optional[TNode]: - """The least common ancestor of all anchor nodes.""" - paths = self._apaths - path0 = min(paths, key=len) - indices = range(len(path0)) - if i := bisect( - indices, - False, - key=lambda ind: any(nid(path0[ind]) != nid(p[ind]) for p in paths), - ): - lca = self._lca = path0[i - 1] - return lca - else: - return None - - @lru_cache - def _common2(self, i, j) -> int: - path_i, path_j = self._apaths[i], self._apaths[j] - indices = range(min(len(path_i), len(path_j))) - return bisect(indices, False, key=lambda ind: nid(path_i[ind]) != nid(path_j[ind])) - 1 + def lca(self) -> Optional[T]: + try: + i = min(self._ancestor_levels, default=0) + return self._anchor_paths[0][i] + except (IndexError, ValueError): + return None # Perhaps this is a bit dirty -class RouteView(Sized, metaclass=ABCMeta): - def __init__(self, route, apaths): +class EdgesView: + """View of edges of this route.""" + def __init__(self, route: Route): self._route = route - self._apaths = apaths - - def count(self): - # Counting takes logarithmic time, so we define len in subclasses - return len(self) - - -class AnchorsView(RouteView): - def __len__(self): - return len(self._apaths) - - def __getitem__(self, item): - return self._apaths[item][-1] - -class NodesView(RouteView): def __iter__(self): - indices = range(len(self._apaths)) - path_j = None - for i, j in itertools.pairwise(indices): - path_i, path_j = self._apaths[i : j + 1] - c = self._route._common2(i, j) - yield from path_i[:c:-1] + path_j[c:-1] - if path_j: - yield path_j[-1] + return itertools.pairwise(self._route) def __reversed__(self): - indices = range(len(self._apaths)) - path_i = None - for j, i in itertools.pairwise(indices[::-1]): - path_i, path_j = self._apaths[i : j + 1] - c = self._route._common2(i, j) - yield from path_j[:c:-1] + path_i[c:-1] - if path_i: - yield path_i[-1] - - def __len__(self): - s = 1 - indices = range(len(self._apaths)) - for i, j in itertools.pairwise(indices): - p1, p2 = self._apaths[i : j + 1] - s += len(p1) + len(p2) - 2 * self._route._common2(i, j) - 2 - return s - - -class EdgesView(RouteView): - def __iter__(self): - return itertools.pairwise(self._route.nodes) + return ((x, y) for (y, x) in itertools.pairwise(reversed(self._route))) - def __reversed__(self): - return itertools.pairwise(reversed(self._route.nodes)) + def __len__(self) -> int: + n = len(self._route) + if n > 0: + return n - 1 + else: + return 0 + + count = __len__ - def __len__(self): - return len(self._route.nodes) - 1 +def _common2(path_i, path_j) -> int: + indices = range(min(len(path_i), len(path_j))) + return bisect(indices, False, key=lambda ind: nid(path_i[ind]) != nid(path_j[ind])) - 1 diff --git a/tests/test_route.py b/tests/test_route.py index 91c2ded..fde726f 100644 --- a/tests/test_route.py +++ b/tests/test_route.py @@ -14,6 +14,8 @@ def setUp(self): llrr = llr.children[1] self.tree = tree + self.route0 = Route() + self.route_solo = Route(tree) self.route1 = Route(tree, llrr, tree) self.route2 = Route(llrr, r, llrr, r) self.route3 = Route(r, r, tree, tree) @@ -22,6 +24,14 @@ def setUp(self): self.heap_route = Route(HEAP_TREE.children[0], HEAP_TREE.children[1]) def test_anchors(self): + result = [node.value for node in self.route0] + self.assertEqual([], result) + self.assertEqual(0, len(self.route0.anchors)) + + result = [node.value for node in self.route_solo] + self.assertEqual([0], result) + self.assertEqual(1, len(self.route_solo.anchors)) + result = [node.value for node in self.route1.anchors] self.assertEqual([0, 18, 0], result) self.assertEqual(3, len(self.route1.anchors)) @@ -43,6 +53,14 @@ def test_anchors(self): self.assertEqual(3, len(self.route4.anchors)) def test_nodes(self): + result = [node.value for node in self.route0.nodes] + self.assertEqual([], result) + self.assertEqual(0, len(self.route0.nodes)) + + result = [node.value for node in self.route_solo.nodes] + self.assertEqual([0], result) + self.assertEqual(1, len(self.route_solo.nodes)) + result = [node.value for node in self.route1.nodes] self.assertEqual([0, 1, 3, 8, 18, 8, 3, 1, 0], result) self.assertEqual(9, len(self.route1.nodes)) @@ -64,6 +82,16 @@ def test_nodes(self): self.assertEqual(3, len(self.heap_route.nodes)) def test_edges(self): + result = [(v1.value, v2.value) for (v1, v2) in self.route0.edges] + expected = [] + self.assertEqual(expected, result) + self.assertEqual(0, len(self.route0.edges)) + + result = [(v1.value, v2.value) for (v1, v2) in self.route_solo.edges] + expected = [] + self.assertEqual(expected, result) + self.assertEqual(0, len(self.route_solo.edges)) + result = [(v1.value, v2.value) for (v1, v2) in self.route1.edges] expected = [(0, 1), (1, 3), (3, 8), (8, 18), (18, 8), (8, 3), (3, 1), (1, 0)] self.assertEqual(expected, result) @@ -74,26 +102,45 @@ def test_reversed(self): edge_result = [(v1.value, v2.value) for (v1, v2) in reversed(self.route2.edges)] node_expected = [2, 0, 1, 3, 8, 18, 8, 3, 1, 0, 2, 0, 1, 3, 8, 18] edge_expected = [ - (2, 0), - (0, 1), - (1, 3), - (3, 8), - (8, 18), - (18, 8), - (8, 3), - (3, 1), - (1, 0), (0, 2), - (2, 0), - (0, 1), - (1, 3), - (3, 8), + (1, 0), + (3, 1), + (8, 3), + (18, 8), (8, 18), + (3, 8), + (1, 3), + (0, 1), + (2, 0), + (0, 2), + (1, 0), + (3, 1), + (8, 3), + (18, 8), ] self.assertEqual(node_expected, node_result) self.assertEqual(edge_expected, edge_result) + def test_reversed_edge_cases(self): + node_result0 = [node.value for node in reversed(self.route0.nodes)] + edge_result0 = [(v1.value, v2.value) for (v1, v2) in reversed(self.route0.edges)] + + node_result_solo = [node.value for node in reversed(self.route_solo.nodes)] + edge_result_solo = [(v1.value, v2.value) for (v1, v2) in reversed(self.route_solo.edges)] + + + node_expected0 = [] + node_expected_solo = [0] + edge_expected = [ ] + + self.assertEqual(node_expected0, node_result0) + self.assertEqual(node_expected_solo, node_result_solo) + self.assertEqual(edge_expected, edge_result0) + self.assertEqual(edge_expected, edge_result_solo) + def test_lca(self): + self.assertIsNone(self.route0.lca) + self.assertEqual(0, self.route_solo.lca.value) self.assertEqual(0, self.route1.lca.value) self.assertEqual(0, self.route2.lca.value) self.assertEqual(0, self.route3.lca.value) From 18c2b41eb3a12d009c1af58b0f64dbd59f93c3af Mon Sep 17 00:00:00 2001 From: Verweijen Date: Mon, 4 Aug 2025 16:09:14 +0200 Subject: [PATCH 2/2] Make path more routelike --- docs/source/CHANGES.md | 12 +++++++++++ src/abstracttree/mixins/views.py | 10 ++++++++- src/abstracttree/route.py | 35 +++++++++++++++++++++----------- tests/test_route.py | 9 ++++---- 4 files changed, 48 insertions(+), 18 deletions(-) diff --git a/docs/source/CHANGES.md b/docs/source/CHANGES.md index 0343a64..f52f560 100644 --- a/docs/source/CHANGES.md +++ b/docs/source/CHANGES.md @@ -1,5 +1,17 @@ # Changelog +## Version 0.2.1 + +* Improve and simplify Route: + * Handle edge-cases correctly, especially when dealing with 1 of 0 anchors. + These used to return incorrect values for e.g. `route.count()`. + * Merge `Route` and `NodesView` classes. + * Make anchors return a `tuple` instead of an `AnchorView`. + * Make `reversed(route.edges)` return edges in parent-child order for consistency with `iter(route.edges)`. + In prior versions, `reversed(path)` would return edges in child-parent order. +* Implement `path.edges` as well to make `path` more `routelike`. +* Implement `path.to` as a shortcut to create a `Route` by writing `route = node.path.to(other_node)`. + ## Version 0.2.0 * Add generics `TreeLike` and `DownTreeLike`. diff --git a/src/abstracttree/mixins/views.py b/src/abstracttree/mixins/views.py index d571b89..8d313c9 100644 --- a/src/abstracttree/mixins/views.py +++ b/src/abstracttree/mixins/views.py @@ -4,13 +4,14 @@ from typing import Iterable, TypeVar from .. import iterators as _iterators +from ..route import EdgesView, Route T = TypeVar("T", bound="Tree") class TreeView(Iterable[T], metaclass=ABCMeta): __slots__ = "_node" - itr_method = None + itr_method = None # TODO Should __init_subclass__ be used instead? def __init__(self, node: T): self._node: T = node @@ -52,9 +53,16 @@ def __contains__(self, item): def __reversed__(self): return _iterators.path(self._node, reverse=True) + def to(self, other: T): + return Route(self, other) + def count(self): return _ilen(reversed(self)) + @property + def edges(self): + return EdgesView(self) + class NodesView(TreeView): """View over nodes.""" diff --git a/src/abstracttree/route.py b/src/abstracttree/route.py index 99bc473..4100073 100644 --- a/src/abstracttree/route.py +++ b/src/abstracttree/route.py @@ -1,7 +1,7 @@ import itertools from bisect import bisect -from collections.abc import Sequence, MutableSequence -from typing import TypeVar, Optional, Generic +from collections.abc import Sequence, MutableSequence, Iterable, Collection +from typing import TypeVar, Optional, Iterator from abstracttree import iterators as _iterators from abstracttree.generics import TreeLike, nid @@ -9,7 +9,7 @@ T = TypeVar("T", bound=TreeLike) -class Route(Generic[T]): +class Route(Iterable[T]): """Representation of a route trough adjacent nodes in the tree. Two nodes are adjacent if they have a parent-child relationship. @@ -48,8 +48,10 @@ def add_anchor(self, anchor: T): self._ancestor_levels.append(_common2(last_path, anchor_path)) self._anchor_paths.append(anchor_path) + assert len(self._anchor_paths) == len(self._ancestor_levels) + 1 - def __iter__(self): + def __iter__(self) -> Iterator[T]: + """Iterate over nodes on route.""" if len(self._anchor_paths) < 2: yield from self.anchors path_j = None @@ -59,7 +61,8 @@ def __iter__(self): if path_j is not None: yield path_j[-1] - def __reversed__(self): + def __reversed__(self) -> Iterator[T]: + """Reversed iterator over nodes.""" if len(self._anchor_paths) < 2: yield from self.anchors path_j = None @@ -70,7 +73,11 @@ def __reversed__(self): if path_j is not None: yield path_j[-1] + def __bool__(self): + return bool(self._anchor_paths) + def __len__(self) -> int: + """How many nodes are on route?""" p, l = self._anchor_paths, self._ancestor_levels if len(p) < 2: return len(p) @@ -79,7 +86,7 @@ def __len__(self) -> int: count = __len__ @property - def anchors(self): + def anchors(self) -> Collection[T]: """View of the anchor nodes.""" return [path[-1] for path in self._anchor_paths] @@ -95,16 +102,21 @@ def edges(self): @property def lca(self) -> Optional[T]: + """Find node that is the common ancestor of nodes on path.""" try: i = min(self._ancestor_levels, default=0) return self._anchor_paths[0][i] except (IndexError, ValueError): + # TODO Raise exception or return None? return None # Perhaps this is a bit dirty -class EdgesView: +class EdgesView(Iterable[tuple[T, T]]): """View of edges of this route.""" - def __init__(self, route: Route): + __slots__ = "_route" + + def __init__(self, route): + # Note: route can be either Route or mixins.views.Path self._route = route def __iter__(self): @@ -113,15 +125,14 @@ def __iter__(self): def __reversed__(self): return ((x, y) for (y, x) in itertools.pairwise(reversed(self._route))) - def __len__(self) -> int: - n = len(self._route) - if n > 0: + def count(self) -> int: + if n := self._route.count(): return n - 1 else: return 0 - count = __len__ def _common2(path_i, path_j) -> int: + # TODO Maybe call this method prefix_length indices = range(min(len(path_i), len(path_j))) return bisect(indices, False, key=lambda ind: nid(path_i[ind]) != nid(path_j[ind])) - 1 diff --git a/tests/test_route.py b/tests/test_route.py index fde726f..40e3b9c 100644 --- a/tests/test_route.py +++ b/tests/test_route.py @@ -85,17 +85,17 @@ def test_edges(self): result = [(v1.value, v2.value) for (v1, v2) in self.route0.edges] expected = [] self.assertEqual(expected, result) - self.assertEqual(0, len(self.route0.edges)) + self.assertEqual(0, self.route0.edges.count()) result = [(v1.value, v2.value) for (v1, v2) in self.route_solo.edges] expected = [] self.assertEqual(expected, result) - self.assertEqual(0, len(self.route_solo.edges)) + self.assertEqual(0, self.route_solo.edges.count()) result = [(v1.value, v2.value) for (v1, v2) in self.route1.edges] expected = [(0, 1), (1, 3), (3, 8), (8, 18), (18, 8), (8, 3), (3, 1), (1, 0)] self.assertEqual(expected, result) - self.assertEqual(8, len(self.route1.edges)) + self.assertEqual(8, self.route1.edges.count()) def test_reversed(self): node_result = [node.value for node in reversed(self.route2.nodes)] @@ -128,10 +128,9 @@ def test_reversed_edge_cases(self): node_result_solo = [node.value for node in reversed(self.route_solo.nodes)] edge_result_solo = [(v1.value, v2.value) for (v1, v2) in reversed(self.route_solo.edges)] - node_expected0 = [] node_expected_solo = [0] - edge_expected = [ ] + edge_expected = [] self.assertEqual(node_expected0, node_result0) self.assertEqual(node_expected_solo, node_result_solo)