Improve route code

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`.

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."""

src/abstracttree/route.py

@@ -1,146 +1,138 @@
 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, Iterable, Collection
+from typing import TypeVar, Optional, Iterator
 
-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(Iterable[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)
+        assert len(self._anchor_paths) == len(self._ancestor_levels) + 1
+
+    def __iter__(self) -> Iterator[T]:
+        """Iterate over nodes on route."""
+        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) -> Iterator[T]:
+        """Reversed iterator over nodes."""
+        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 __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)
+        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):
+    def anchors(self) -> Collection[T]:
         """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
-
-
-class RouteView(Sized, metaclass=ABCMeta):
-    def __init__(self, route, apaths):
+    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(Iterable[tuple[T, T]]):
+    """View of edges of this route."""
+    __slots__ = "_route"
+
+    def __init__(self, route):
+        # Note: route can be either Route or mixins.views.Path
         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 count(self) -> int:
+        if n := self._route.count():
+            return n - 1
+        else:
+            return 0
 
-    def __reversed__(self):
-        return itertools.pairwise(reversed(self._route.nodes))
 
-    def __len__(self):
-        return len(self._route.nodes) - 1
+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