From 5b5b1e3ec44c9160f915236d78d93f6dba06a089 Mon Sep 17 00:00:00 2001 From: Peter Corke Date: Mon, 12 Oct 2026 10:01:12 +1000 Subject: [PATCH 1/3] feat: one Vertex class; Graph(directed=...) chooses the kind of graph UVertex and DVertex differed only in four private methods that recorded an edge at its ends, which is the graph's decision, not the vertex's: a vertex has no direction of its own. Having the kind in both the graph and its vertices meant keeping them consistent, with checks (add_vertex's kind test, vertex_class's kind test) whose only job was to refuse the combinations the design allowed to be written. - the hooks _attach, _detach, _incoming and _endpoints_match move to the graph kinds: UGraph records an edge at both ends, DGraph as leaving its start and entering its end. Vertex.connect calls the graph's hook - there is one vertex class, Vertex. UVertex and DVertex remain as deprecated names for it (module __getattr__, DeprecationWarning), so existing code that subclasses or constructs them runs; they are not in __all__ - Graph is concrete: Graph(directed=True) is a DGraph and Graph() a UGraph, so the kind can be a variable; Graph.Adjacency and Graph.Dict take directed= the same way. UGraph(directed=True) is a ValueError. isinstance, the hooks and pickling see the honest subclass - Graph.directed reports the kind, replacing the isinstance tests in Adjacency, iscyclic, incidence, validate, the exporters and Edge.__str__ - vertex_class need only be a subclass of Vertex Behaviour change: add_vertex no longer has a wrong-kind vertex to refuse, and a bare Vertex, which could not join any graph, now can. An Edge between standalone vertices has no kind to show, so its str uses -- not ->. Tests: the hook tests exercise the graph hooks; test_unified_graph covers the dispatch, the aliases and the kind-aware constructors. Downstream, MVTB's bundle-adjust tests and RTB's mobile and planner tests pass through the aliases. Co-Authored-By: Claude Sonnet 5.5 --- docs/source/dvertex.rst | 9 - docs/source/uvertex.rst | 9 - src/pgraph/PGraph.py | 546 +++++++++++++++++--------------- src/pgraph/__init__.py | 20 +- src/pgraph/exporters.py | 4 +- tests/test_add_edge_edge.py | 4 +- tests/test_add_vertex_names.py | 8 +- tests/test_adjacent.py | 14 +- tests/test_attribute_hygiene.py | 11 +- tests/test_detach_hook.py | 38 ++- tests/test_graph.py | 28 +- tests/test_graph_classes.py | 27 +- tests/test_pickle.py | 13 +- tests/test_unified_graph.py | 210 ++++++++++++ tests/test_validate.py | 8 +- tests/test_vertex_hook.py | 91 +++--- 16 files changed, 635 insertions(+), 405 deletions(-) delete mode 100644 docs/source/dvertex.rst delete mode 100644 docs/source/uvertex.rst create mode 100644 tests/test_unified_graph.py diff --git a/docs/source/dvertex.rst b/docs/source/dvertex.rst deleted file mode 100644 index 10e4d49f..00000000 --- a/docs/source/dvertex.rst +++ /dev/null @@ -1,9 +0,0 @@ -Directed graph vertex ---------------------- - -``DVertex`` overrides only ``connect()`` -- see :doc:`common` for the full -method list (adjacent, edges, distance, degree, removal, ...). - -.. autoclass:: pgraph.PGraph.DVertex - :members: connect - :show-inheritance: diff --git a/docs/source/uvertex.rst b/docs/source/uvertex.rst deleted file mode 100644 index 8c5a42f4..00000000 --- a/docs/source/uvertex.rst +++ /dev/null @@ -1,9 +0,0 @@ -Undirected graph vertex ------------------------ - -``UVertex`` overrides only ``connect()`` -- see :doc:`common` for the full -method list (adjacent, edges, distance, degree, removal, ...). - -.. autoclass:: pgraph.PGraph.UVertex - :members: connect - :show-inheritance: diff --git a/src/pgraph/PGraph.py b/src/pgraph/PGraph.py index a314ea1d..f6e57011 100644 --- a/src/pgraph/PGraph.py +++ b/src/pgraph/PGraph.py @@ -48,10 +48,16 @@ def _SE2(v: NDArray) -> float: class Graph(ABC): - #: concrete vertex type for this graph kind, provided by :class:`UGraph` - #: and :class:`DGraph` -- lets :meth:`add_vertex` and :meth:`vertex_copy` - #: be defined once here rather than duplicated per subclass. - _vertex_cls: ClassVar[type[Vertex]] + #: whether the edges of this kind of graph are directed, set by + #: :class:`UGraph` and :class:`DGraph` + _directed: ClassVar[bool] + + def __new__(cls, *args: Any, directed: bool | None = None, **kwargs: Any) -> Graph: + # Graph(directed=True) is a DGraph and Graph() a UGraph; the classes + # underneath stay honest, so isinstance and the hooks work as usual + if cls is Graph: + cls = DGraph if directed else UGraph + return super().__new__(cls) def __init__( self, @@ -62,9 +68,10 @@ def __init__( dim: int | None = None, vertex_class: type[Vertex] | None = None, edge_class: type[Edge] | None = None, + directed: bool | None = None, ): """ - Abstract base class for graphs + Graph, undirected or directed :param metric: distance metric, defaults to "L2" :type metric: callable or str, optional @@ -78,18 +85,31 @@ def __init__( length, or none at all) :type dim: int, optional :param vertex_class: class of the vertices this graph creates, defaults - to ``UVertex`` for a :class:`UGraph` and ``DVertex`` for a - :class:`DGraph` - :type vertex_class: subclass of ``UVertex`` or ``DVertex``, optional + to :class:`Vertex` + :type vertex_class: subclass of :class:`Vertex`, optional :param edge_class: class of the edges this graph creates, defaults to :class:`Edge` :type edge_class: subclass of ``Edge``, optional :raises ValueError: ``dim`` is given but is not a positive integer - :raises TypeError: ``vertex_class`` is not a subclass of this graph's - own vertex kind, or ``edge_class`` is not a subclass of ``Edge`` + :param directed: whether the edges are directed, defaults to False + for ``Graph`` and is fixed by the class for :class:`UGraph` and + :class:`DGraph` + :type directed: bool, optional + :raises ValueError: ``directed`` contradicts the class, for example + ``UGraph(directed=True)`` + :raises TypeError: ``vertex_class`` is not a subclass of + :class:`Vertex`, or ``edge_class`` is not a subclass of ``Edge`` - This is the common base class of :class:`UGraph` and :class:`DGraph` - and should not be instantiated directly. + ``Graph(directed=True)`` is a :class:`DGraph` and ``Graph()`` a + :class:`UGraph`, so the kind can be chosen by a variable:: + + g = Graph(directed=is_directed) + + :class:`UGraph` and :class:`DGraph` remain, and are what + ``isinstance`` and the documentation refer to. Everything else, the + vertices included, is the same for both kinds, so there is one vertex + class, :class:`Vertex`. A vertex has no direction of its own; the + graph it belongs to decides how an edge is recorded at its ends. All arguments are keyword-only, so ``UGraph(dim=6)`` is correct and ``UGraph(6)`` raises :exc:`TypeError`. @@ -118,14 +138,16 @@ def __init__( """ if dim is not None and dim <= 0: raise ValueError(f"dim must be a positive integer, got {dim!r}") - kind = type(self)._vertex_cls - if vertex_class is None: - vertex_class = kind - elif not (isinstance(vertex_class, type) and issubclass(vertex_class, kind)): - raise TypeError( - f"vertex_class must be a subclass of {kind.__name__} for a " - f"{type(self).__name__}, got {vertex_class!r}" + if directed is not None and bool(directed) != type(self)._directed: + raise ValueError( + f"{type(self).__name__} is " + f"{'directed' if type(self)._directed else 'undirected'}, " + f"but directed={directed!r} was given" ) + if vertex_class is None: + vertex_class = Vertex + elif not (isinstance(vertex_class, type) and issubclass(vertex_class, Vertex)): + raise TypeError(f"vertex_class must be a subclass of Vertex, got {vertex_class!r}") if edge_class is None: edge_class = Edge elif not (isinstance(edge_class, type) and issubclass(edge_class, Edge)): @@ -310,7 +332,7 @@ def Adjacency( coord = coords[i, :] g.add_vertex(name=name, coord=coord) - if isinstance(g, UGraph): + if not g.directed: # undirected graph: upper triangle including the diagonal for i in range(A.shape[0]): for j in range(i, A.shape[1]): @@ -325,22 +347,108 @@ def Adjacency( return g + @property + def directed(self) -> bool: + """ + Test if the graph is directed + + :return: True for a :class:`DGraph`, False for a :class:`UGraph` + :rtype: bool + + An undirected edge is recorded at both of its vertices, and is + followed in either direction. A directed edge is recorded as leaving + its start vertex and entering its end vertex. + + .. runblock:: pycon + + >>> from pgraph import Graph + >>> Graph().directed + >>> Graph(directed=True).directed + + :seealso: :class:`UGraph` :class:`DGraph` + """ + return self._directed + + # the hooks: the one place where undirected and directed graphs differ in + # how an edge is held. A vertex has no direction of its own, so its + # connect() and the graph's remove_edge() ask the graph. + + def _attach(self, edge: Edge) -> None: + """ + Record a new edge in the edge lists of its vertices (private method) + + :param edge: the edge just created or supplied to + :meth:`Vertex.connect`, with ``edge.v1`` and ``edge.v2`` set and + already part of the graph + :raises NotImplementedError: not provided by this graph kind + + This is the hook that :meth:`Vertex.connect` calls once the connection + has been validated and the edge registered with the graph, so it is + never called for a connection that is refused. :class:`UGraph` + records the edge at both ends and :class:`DGraph` as leaving its start + and entering its end. It is defined by the graph kind, not by + :meth:`Vertex.connect`. + """ + raise NotImplementedError( + f"{type(self).__name__} does not define how an edge is attached; " + "subclass UGraph or DGraph" + ) + + def _detach(self, edge: Edge) -> None: + """ + Remove an edge from the edge lists of its vertices (private method) + + :param edge: the edge being removed, with ``edge.v1`` and ``edge.v2`` + still set + :raises NotImplementedError: not provided by this graph kind + + The twin of :meth:`_attach`: it undoes exactly what that recorded. + Called by :meth:`remove_edge` before the edge's endpoints are + cleared. An edge that is absent from a list is skipped rather than + treated as an error. + """ + raise NotImplementedError( + f"{type(self).__name__} does not define how an edge is detached; " + "subclass UGraph or DGraph" + ) + + def _incoming(self, vertex: Vertex) -> list[Edge]: + """ + The edges that end at a vertex (private method) + + :param vertex: a vertex of this graph + :raises NotImplementedError: not provided by this graph kind + """ + raise NotImplementedError( + f"{type(self).__name__} does not define the incoming edges; " + "subclass UGraph or DGraph" + ) + + def _endpoints_match(self, edge: Edge, v1: Vertex, v2: Vertex) -> bool: + """ + Test if an edge already connects two vertices (private method) + + A directed edge must run from ``v1`` to ``v2``; see + :meth:`UGraph._endpoints_match` for the undirected relaxation. + """ + return edge.v1 is v1 and edge.v2 is v2 + @property def vertex_class(self) -> type[Vertex]: """ Class of the vertices this graph creates :return: vertex class - :rtype: subclass of ``UVertex`` or ``DVertex`` + :rtype: subclass of ``Vertex`` This is read-only. It is set by the ``vertex_class`` argument of the - graph constructor, see :meth:`__init__`, and is ``UVertex`` for a - :class:`UGraph` and ``DVertex`` for a :class:`DGraph` if not given. + graph constructor, see :meth:`__init__`, and is :class:`Vertex` if not + given. .. runblock:: pycon - >>> from pgraph import UGraph, UVertex - >>> class MyVertex(UVertex): + >>> from pgraph import UGraph, Vertex + >>> class MyVertex(Vertex): ... pass >>> g = UGraph(vertex_class=MyVertex) >>> print(g.vertex_class.__name__) @@ -424,7 +532,7 @@ def validate(self) -> list[str]: from collections import Counter problems: list[str] = [] - directed = isinstance(self, DGraph) + directed = self.directed # ---- the containers themselves must hold the right kind of thing; a # stray object is reported and set aside so the checks below can run @@ -694,7 +802,7 @@ def __setstate__(self, state: dict[str, Any]) -> None: v1, v2 = self._vertexlist[i], self._vertexlist[j] e._set_endpoints(v1, v2) self._edges[e] = None - v1._attach(e, v2) + self._attach(e) def add_vertex( self, coord: ArrayLike | Vertex | None = None, name: str | None = None @@ -703,8 +811,8 @@ def add_vertex( Add a vertex to the graph :param coord: coordinate for an embedded graph, or an existing vertex - of this graph's own kind (``UVertex`` for :class:`UGraph`, - ``DVertex`` for :class:`DGraph`) to add as-is, defaults to None + (an instance of this graph's :attr:`vertex_class`, not belonging + to a graph) to add as-is, defaults to None :type coord: array-like or Vertex subclass, optional :param name: name of vertex, defaults to "#i" :type name: str, optional @@ -722,8 +830,7 @@ def add_vertex( - ``g.add_vertex()`` creates a new vertex with optional ``coord`` and ``name``. - ``g.add_vertex(v)`` takes an instance of this graph's - :attr:`vertex_class` (by default any ``UVertex`` for a - :class:`UGraph`, any ``DVertex`` for a :class:`DGraph`) and adds it + :attr:`vertex_class` (by default any :class:`Vertex`) and adds it to the graph. If the vertex has no name and ``name`` is None give it a default name @@ -743,13 +850,13 @@ def add_vertex( .. runblock:: pycon - >>> from pgraph import UGraph, UVertex + >>> from pgraph import UGraph, Vertex >>> g = UGraph() >>> v1 = g.add_vertex(coord=[0,0]) >>> repr(v1) >>> print(v1) >>> print(v1.name) - >>> v2 = g.add_vertex(UVertex(coord=[1,1], name='v2')) + >>> v2 = g.add_vertex(Vertex(coord=[1,1], name='v2')) >>> print(v2.name) >>> g['v2'] @@ -828,7 +935,7 @@ def vertex_copy(cls, vertex: Vertex) -> Vertex: A vertex can only belong to a single graph, so this method is used to create a new vertex with the same name and coordinates for inclusion - in a new graph -- of ``cls``'s own vertex kind, per :attr:`_vertex_cls`. + in a new graph. A vertex is the same for undirected and directed graphs. .. runblock:: pycon @@ -840,7 +947,7 @@ def vertex_copy(cls, vertex: Vertex) -> Vertex: :seealso: :meth:`Vertex.copy` """ - return cls._vertex_cls(coord=vertex.coord, name=vertex.name) + return Vertex(coord=vertex.coord, name=vertex.name) def _resolve_vertex(self, v: Vertex | str, label: str) -> Vertex: """ @@ -1045,7 +1152,7 @@ def remove_edge(self, edge: Edge) -> None: # how the edge is held differs between undirected and directed # vertices, so the vertex kind undoes what its _attach did - v1._detach(edge, v2) + self._detach(edge) v1._connectivitychange = True v2._connectivitychange = True @@ -2018,8 +2125,8 @@ def dotfile( .. note:: - - The graph is undirected if it is a subclass of ``UGraph`` - - The graph is directed if it is a subclass of ``DGraph`` + - The graph is undirected if it is a :class:`UGraph` + - The graph is directed if it is a :class:`DGraph` .. note:: The ``graphviz`` formatters ``dot`` and ``neato`` can be used to render the graph in various formats including PDF, PNG, JPG, GIF, SVG, etc. The formmatters are @@ -2129,7 +2236,7 @@ def iscyclic(self) -> bool: :seealso: :meth:`adjacency` """ - if isinstance(self, UGraph): + if not self.directed: return self.ne > self.n - self.nc # DGraph: Kahn's algorithm @@ -2463,7 +2570,7 @@ def incidence(self) -> NDArray: assert e.v1 is not None and e.v2 is not None I[vdict[e.v1], j] = 1 I[vdict[e.v2], j] = 1 - if e.v1 is e.v2 and isinstance(self, UGraph): + if e.v1 is e.v2 and not self.directed: I[vdict[e.v1], j] = 2 # self-loop, counted at both ends return I @@ -2947,16 +3054,57 @@ class UGraph(Graph): """ Class for undirected graphs - Vertices are :class:`UVertex` objects, or instances of the subclass given + Vertices are :class:`Vertex` objects, or instances of the subclass given as ``vertex_class``. Edges are :class:`Edge` objects, or instances of the subclass given as ``edge_class``; see :class:`Graph` for the constructor arguments and the exceptions they raise. + An edge is recorded at *both* of its vertices, so it is found from either + end, and an edge supplied to :meth:`Vertex.connect` may have its endpoints + in either order. + .. inheritance-diagram:: UGraph :seealso: :class:`Graph` :class:`DGraph` """ + _directed = False + + def _attach(self, edge: Edge) -> None: + """ + Record an undirected edge at both ends (private method) + + The edge is added to *both* vertices' edge lists, so it is discovered + from either end. A self-loop is therefore held twice by its vertex, + which is why it counts 2 towards the degree. + """ + assert edge.v1 is not None and edge.v2 is not None + edge.v1._edgelist.append(edge) + edge.v2._edgelist.append(edge) + + def _detach(self, edge: Edge) -> None: + """ + Remove an undirected edge from both ends (private method) + + A self-loop was recorded twice by :meth:`_attach`, so it is removed + twice. + """ + for vertex in (edge.v1, edge.v2): + if vertex is not None and edge in vertex._edgelist: + vertex._edgelist.remove(edge) + + def _incoming(self, vertex: Vertex) -> list[Edge]: + """The edges that end here: for an undirected graph, all of them""" + return vertex._edgelist + + def _endpoints_match(self, edge: Edge, v1: Vertex, v2: Vertex) -> bool: + """ + Test if an edge already connects two vertices (private method) + + An undirected edge has no direction, so either order is accepted. + """ + return (edge.v1 is v1 and edge.v2 is v2) or (edge.v1 is v2 and edge.v2 is v1) + def _graphcolor(self) -> None: """ Color the graph @@ -3006,16 +3154,46 @@ class DGraph(Graph): """ Class for directed graphs - Vertices are :class:`DVertex` objects, or instances of the subclass given + Vertices are :class:`Vertex` objects, or instances of the subclass given as ``vertex_class``. Edges are :class:`Edge` objects, or instances of the subclass given as ``edge_class``; see :class:`Graph` for the constructor arguments and the exceptions they raise. + An edge is recorded as leaving its start vertex and entering its end + vertex, and an edge supplied to :meth:`Vertex.connect` must run exactly + from that vertex to the other. + .. inheritance-diagram:: DGraph :seealso: :class:`Graph` :class:`UGraph` """ + _directed = True + + def _attach(self, edge: Edge) -> None: + """ + Record a directed edge at both ends (private method) + + The edge goes in the start vertex's list of outgoing edges and in the + end vertex's list of incoming ones. + """ + assert edge.v1 is not None and edge.v2 is not None + edge.v1._edgelist.append(edge) + edge.v2._inlist.append(edge) + + def _detach(self, edge: Edge) -> None: + """ + Remove a directed edge from its start and from its end (private method) + """ + if edge.v1 is not None and edge in edge.v1._edgelist: + edge.v1._edgelist.remove(edge) + if edge.v2 is not None and edge in edge.v2._inlist: + edge.v2._inlist.remove(edge) + + def _incoming(self, vertex: Vertex) -> list[Edge]: + """The edges that end at the vertex""" + return vertex._inlist + def _graphcolor(self) -> int | None: """ Color the graph @@ -3260,9 +3438,9 @@ def __repr__(self) -> str: .. runblock:: pycon - >>> from pgraph import UVertex, Edge - >>> v1 = UVertex(coord=[1,2], name="A") - >>> v2 = UVertex(coord=[3,4], name="B") + >>> from pgraph import Vertex, Edge + >>> v1 = Vertex(coord=[1,2], name="A") + >>> v2 = Vertex(coord=[3,4], name="B") >>> e = Edge(v1, v2, cost=5.0, data="A to B") >>> repr(e) @@ -3279,15 +3457,16 @@ def __str__(self) -> str: .. runblock:: pycon - >>> from pgraph import UVertex, Edge - >>> v1 = UVertex(coord=[1,2], name="A") - >>> v2 = UVertex(coord=[3,4], name="B") + >>> from pgraph import Vertex, Edge + >>> v1 = Vertex(coord=[1,2], name="A") + >>> v2 = Vertex(coord=[3,4], name="B") >>> e = Edge(v1, v2, cost=5.0, data="A to B") >>> str(e) :seealso: :meth:`__repr__` """ - arrow = "->" if isinstance(self.v1, DVertex) else "--" + graph = self.v1._graph if self.v1 is not None else None + arrow = "->" if graph is not None and graph.directed else "--" cost_str = "None" if self.cost is None else f"{self.cost:.4g}" s = f"{self.__class__.__name__}{{{self.v1} {arrow} {self.v2}, cost={cost_str}}}" if self.data is not None: @@ -3317,7 +3496,7 @@ def connect(self, v1: Vertex, v2: Vertex) -> None: .. runblock:: pycon - >>> from pgraph import UVertex, Edge, UGraph + >>> from pgraph import Vertex, Edge, UGraph >>> g = UGraph() >>> v1 = g.add_vertex(coord=[0,0], name='v1') >>> v2 = g.add_vertex(coord=[1,1], name='v2') @@ -3359,9 +3538,9 @@ def next(self, vertex: Vertex) -> Vertex: .. runblock:: pycon - >>> from pgraph import UVertex, Edge - >>> v1 = UVertex(coord=[1,2], name="A") - >>> v2 = UVertex(coord=[3,4], name="B") + >>> from pgraph import Vertex, Edge + >>> v1 = Vertex(coord=[1,2], name="A") + >>> v2 = Vertex(coord=[3,4], name="B") >>> e = Edge(v1, v2, cost=5.0, data="A to B") >>> print(e) >>> e.next(v1) @@ -3404,9 +3583,9 @@ def endpoints(self) -> list[Vertex]: .. runblock:: pycon - >>> from pgraph import UVertex, Edge - >>> v1 = UVertex(coord=[1,2], name="A") - >>> v2 = UVertex(coord=[3,4], name="B") + >>> from pgraph import Vertex, Edge + >>> v1 = Vertex(coord=[1,2], name="A") + >>> v2 = Vertex(coord=[3,4], name="B") >>> e = Edge(v1, v2, cost=5.0, data="A to B") >>> print(e) >>> print(e.endpoints) @@ -3457,7 +3636,18 @@ def remove(self) -> None: class Vertex: """ - Base class for vertices of directed and non-directed graphs. + A vertex of a graph, directed or undirected. + + A vertex has no direction of its own: how an edge is recorded at its ends + is decided by the graph it belongs to (see :class:`UGraph` and + :class:`DGraph`). So there is one vertex class, which can be subclassed + to give user objects graph capability and passed to the graph as + ``vertex_class``. + + .. versionchanged:: 1.0 + This replaces ``UVertex`` and ``DVertex``, which are deprecated names + for it. They differed only in how an edge was recorded, which is now + the graph's decision. Each vertex has: - ``name`` which is its key in the graph, so renaming a vertex that is @@ -3469,7 +3659,7 @@ class Vertex: and its edges are available from :meth:`edges`, :meth:`out_edges` and :meth:`in_edges`. - :seealso: :class:`UVertex` :class:`DVertex` :class:`Edge` + :seealso: :class:`Graph` :class:`Edge` """ def __init__(self, coord: ArrayLike | None = None, name: str | None = None): @@ -3486,8 +3676,8 @@ def __init__(self, coord: ArrayLike | None = None, name: str | None = None): .. runblock:: pycon - >>> from pgraph import UVertex - >>> v1 = UVertex(coord=[0,0], name='v1') + >>> from pgraph import Vertex + >>> v1 = Vertex(coord=[0,0], name='v1') >>> print(v1) :seealso: :meth:`Graph.add_vertex` @@ -3528,8 +3718,8 @@ def __str__(self) -> str: .. runblock:: pycon - >>> from pgraph import UVertex - >>> v = UVertex(coord=[1,2], name="A") + >>> from pgraph import Vertex + >>> v = Vertex(coord=[1,2], name="A") >>> str(v) """ return f"[{self.name:s}]" @@ -3543,8 +3733,8 @@ def __repr__(self) -> str: .. runblock:: pycon - >>> from pgraph import UVertex - >>> v = UVertex(coord=[1,2], name="A") + >>> from pgraph import Vertex + >>> v = Vertex(coord=[1,2], name="A") >>> repr(v) """ if self.coord is None: @@ -3569,8 +3759,8 @@ def copy(self, cls: type[Graph] | None = None) -> Vertex: .. runblock:: pycon - >>> from pgraph import UVertex, DGraph - >>> v = UVertex(coord=[1,2], name='v1') + >>> from pgraph import Vertex, DGraph + >>> v = Vertex(coord=[1,2], name='v1') >>> v2 = v.copy(cls=DGraph) >>> print(v2) @@ -3743,19 +3933,16 @@ def connect( .. note:: - - If the vertices subclass ``UVertex`` the edge is undirected: it is - recorded in the edge list of *both* vertices, so it is found from - either end. If they subclass ``DVertex`` the edge is directed: it - is recorded only in the edge list of this, the source, vertex. + - In an undirected graph the edge is recorded in the edge list of + *both* vertices, so it is found from either end. In a directed + graph it is recorded as leaving this, the start, vertex and + entering ``dest``. - A supplied ``edge`` built without vertices has its ``v1`` and ``v2`` set to the vertices being connected. One built with vertices must agree with them: in either order if undirected, exactly ``self`` then ``dest`` if directed. - Both vertices must already have been added to the same graph, - e.g. via :meth:`Graph.add_vertex` -- since a graph only ever - accepts its own vertex subclass (see :meth:`UGraph.add_vertex`, - :meth:`DGraph.add_vertex`), this also rules out connecting a - ``UVertex`` to a ``DVertex``. + e.g. via :meth:`Graph.add_vertex`. .. runblock:: pycon @@ -3790,60 +3977,11 @@ def connect( self._graph._connectivitychange = True self._connectivitychange = True - # the one thing that differs between undirected and directed vertices - self._attach(e, dest) + # the one thing that differs between undirected and directed graphs + self._graph._attach(e) return e - def _attach(self, edge: Edge, dest: Vertex) -> None: - """ - Record a new edge in the vertices' edge lists (private method) - - :param edge: the edge just created or supplied by :meth:`connect`, with - ``edge.v1`` and ``edge.v2`` set and already part of the graph - :param dest: the vertex ``connect`` was called with - :raises NotImplementedError: not provided by this vertex kind - - This is the hook that :meth:`connect` calls once the connection has - been validated and the edge registered with the graph, so it is never - called for a connection that is refused. :class:`UVertex` records the - edge at both ends and :class:`DVertex` at the source only. A vertex - kind of your own supplies this, not :meth:`connect`, which is defined - once. - """ - raise NotImplementedError( - f"{type(self).__name__} does not define how an edge is attached; " - "subclass UVertex or DVertex" - ) - - def _detach(self, edge: Edge, dest: Vertex) -> None: - """ - Remove an edge from the vertices' edge lists (private method) - - :param edge: the edge being removed, with ``edge.v1`` and ``edge.v2`` - still set - :param dest: the vertex at the other end, ``edge.v2`` - :raises NotImplementedError: not provided by this vertex kind - - The twin of :meth:`_attach`: it undoes exactly what that recorded. - Called by :meth:`Graph.remove_edge` before the edge's endpoints - are cleared. An edge that is absent from a list is skipped rather - than treated as an error. - """ - raise NotImplementedError( - f"{type(self).__name__} does not define how an edge is detached; " - "subclass UVertex or DVertex" - ) - - def _endpoints_match(self, edge: Edge, dest: Vertex) -> bool: - """ - Test if an edge already connects this vertex to ``dest`` (private method) - - A directed edge must run from this vertex to ``dest``; see - :meth:`UVertex._endpoints_match` for the undirected relaxation. - """ - return edge.v1 is self and edge.v2 is dest - def _bind_edge(self, edge: Edge, dest: Vertex) -> None: """ Make a caller-supplied edge connect this vertex to ``dest`` (private method) @@ -3858,9 +3996,11 @@ def _bind_edge(self, edge: Edge, dest: Vertex) -> None: changed, rather than leaving the graph holding an edge whose endpoints disagree with the vertices' edge lists. """ + graph = self._graph + assert graph is not None # connect() has checked if edge.v1 is None and edge.v2 is None: edge._set_endpoints(self, dest) - elif not self._endpoints_match(edge, dest): + elif not graph._endpoints_match(edge, self, dest): raise ValueError( f"edge already connects {edge.v1} and {edge.v2}, " f"not {self} and {dest}" @@ -4063,7 +4203,9 @@ def in_degree(self) -> int: def _incoming(self) -> list[Edge]: """The edges that end here (private method)""" - return self._inlist + if self._graph is None: + return self._inlist + return self._graph._incoming(self) def out_edges(self) -> list[Edge]: """ @@ -4220,8 +4362,8 @@ def x(self) -> float: .. runblock:: pycon - >>> from pgraph import UVertex - >>> v = UVertex(coord=[1,2], name='v1') + >>> from pgraph import Vertex + >>> v = Vertex(coord=[1,2], name='v1') >>> print(v.x) """ if self.coord is None: @@ -4239,8 +4381,8 @@ def y(self) -> float: .. runblock:: pycon - >>> from pgraph import UVertex - >>> v = UVertex(coord=[1,2], name='v1') + >>> from pgraph import Vertex + >>> v = Vertex(coord=[1,2], name='v1') >>> print(v.y) """ if self.coord is None: @@ -4258,8 +4400,8 @@ def z(self) -> float: .. runblock:: pycon - >>> from pgraph import UVertex - >>> v = UVertex(coord=[1,2,3], name='v1') + >>> from pgraph import Vertex + >>> v = Vertex(coord=[1,2,3], name='v1') >>> print(v.z) """ if self.coord is None: @@ -4327,130 +4469,16 @@ def remove(self) -> None: self._graph.remove_vertex(self) -class UVertex(Vertex): - """ - Vertex subclass for undirected graphs - - This class can be inherited to provide user objects with graph capability. - - :meth:`~Vertex.connect` adds the new edge to *both* vertices' edge - lists, so it is discovered from either end, and accepts a supplied edge - whose endpoints are the two vertices in either order. - - .. runblock:: pycon - - >>> from pgraph import UGraph - >>> g = UGraph() - >>> v1 = g.add_vertex(coord=[0,0], name='v1') - >>> v2 = g.add_vertex(coord=[1,1], name='v2') - >>> v1.connect(v2, cost=1.414) - >>> print(v1.edges()) - >>> print(v2.edges()) - - .. inheritance-diagram:: UVertex - - :seealso: :class:`Vertex` :class:`DVertex` - """ - - def _attach(self, edge: Edge, dest: Vertex) -> None: - """ - Record an undirected edge at both ends (private method) - - The edge is added to *both* vertices' edge lists, so it is discovered - from either end. A self-loop is therefore held twice by its vertex, - which is why it counts 2 towards the degree. - """ - self._edgelist.append(edge) - dest._edgelist.append(edge) - - def _incoming(self) -> list[Edge]: - """The edges that end here: for an undirected vertex, all of them""" - return self._edgelist - - def _detach(self, edge: Edge, dest: Vertex) -> None: - """ - Remove an undirected edge from both ends (private method) - - A self-loop was recorded twice by :meth:`_attach`, so it is removed - twice. - """ - for vertex in (self, dest): - if edge in vertex._edgelist: - vertex._edgelist.remove(edge) - - def _endpoints_match(self, edge: Edge, dest: Vertex) -> bool: - """ - Test if an edge already connects this vertex and ``dest`` (private method) - - An undirected edge has no direction, so either order is accepted. - """ - return (edge.v1 is self and edge.v2 is dest) or ( - edge.v1 is dest and edge.v2 is self +# UVertex and DVertex were two vertex classes that differed only in how an edge +# was recorded at them, which is the graph's decision. They remain as names +# for Vertex, so that existing code that subclasses or constructs them runs. +def __getattr__(name: str) -> Any: + if name in ("UVertex", "DVertex"): + warnings.warn( + f"{name} is deprecated, use Vertex instead: a vertex has no " + "direction of its own, the graph it belongs to decides", + DeprecationWarning, + stacklevel=2, ) - - -class DVertex(Vertex): - """ - Vertex subclass for directed graphs - - This class can be inherited to provide user objects with graph capability. - - :meth:`~Vertex.connect` adds the new edge only to *this* vertex's edge - list, so it is discoverable only from the start of the directed edge, and a - supplied edge must run exactly from this vertex to the other. - - .. runblock:: pycon - - >>> from pgraph import DGraph - >>> g = DGraph() - >>> v1 = g.add_vertex(coord=[0,0], name='v1') - >>> v2 = g.add_vertex(coord=[1,1], name='v2') - >>> v1.connect(v2, cost=1.414) - >>> print(v1.edges()) - >>> print(v2.edges()) - - .. inheritance-diagram:: DVertex - - :seealso: :class:`Vertex` :class:`UVertex` - """ - - def _attach(self, edge: Edge, dest: Vertex) -> None: - """ - Record a directed edge at both ends (private method) - - The edge goes in this vertex's list of outgoing edges and in - ``dest``'s list of incoming ones. - """ - self._edgelist.append(edge) - dest._inlist.append(edge) - - def _detach(self, edge: Edge, dest: Vertex) -> None: - """ - Remove a directed edge from its source and from where it ends - (private method) - """ - if edge in self._edgelist: - self._edgelist.remove(edge) - if edge in dest._inlist: - dest._inlist.remove(edge) - - -# UGraph/DGraph declare their _vertex_cls here, rather than in their own -# class body, because UVertex/DVertex are defined later in this file -- -# _vertex_cls is a real runtime assignment (unlike a type annotation), so it -# can't rely on `from __future__ import annotations` to defer it. -UGraph._vertex_cls = UVertex -DGraph._vertex_cls = DVertex - - -if __name__ == "__main__": - - g = UGraph() - print(g) - - for i in range(10): - g.add_vertex() - - g.add_edge(g[0], g[1]) - - print(g) + return Vertex + raise AttributeError(f"module {__name__!r} has no attribute {name!r}") diff --git a/src/pgraph/__init__.py b/src/pgraph/__init__.py index d7a8aff4..0c3444e3 100644 --- a/src/pgraph/__init__.py +++ b/src/pgraph/__init__.py @@ -1,10 +1,11 @@ +import warnings +from typing import Any + from pgraph.PGraph import ( Graph, Vertex, UGraph, DGraph, - UVertex, - DVertex, Edge, ) @@ -13,7 +14,18 @@ "Vertex", "UGraph", "DGraph", - "UVertex", - "DVertex", "Edge", ] + + +# UVertex and DVertex are deprecated names for Vertex, see pgraph.PGraph +def __getattr__(name: str) -> Any: + if name in ("UVertex", "DVertex"): + warnings.warn( + f"pgraph.{name} is deprecated, use pgraph.Vertex instead: a vertex " + "has no direction of its own, the graph it belongs to decides", + DeprecationWarning, + stacklevel=2, + ) + return Vertex + raise AttributeError(f"module {__name__!r} has no attribute {name!r}") diff --git a/src/pgraph/exporters.py b/src/pgraph/exporters.py index fb6d4f6e..eafa5375 100644 --- a/src/pgraph/exporters.py +++ b/src/pgraph/exporters.py @@ -132,9 +132,7 @@ def format_from_filename(filename: str | os.PathLike | IO | None) -> str | None: def _is_directed(graph: Graph) -> bool: - from .PGraph import DGraph # not at module level: PGraph imports this module - - return isinstance(graph, DGraph) + return graph.directed def _ordered_edges(graph: Graph) -> list[tuple[Edge, Vertex, Vertex]]: diff --git a/tests/test_add_edge_edge.py b/tests/test_add_edge_edge.py index a9ac5a75..bf34aed4 100644 --- a/tests/test_add_edge_edge.py +++ b/tests/test_add_edge_edge.py @@ -6,7 +6,7 @@ import unittest -from pgraph import DGraph, Edge, UGraph, UVertex +from pgraph import DGraph, Edge, UGraph, Vertex class MyEdge(Edge): @@ -87,7 +87,7 @@ def test_vertices_of_another_graph(self): self.assertEqual(h.ne, 0) def test_edge_between_ungraphed_vertices(self): - v, w = UVertex([0, 0]), UVertex([1, 1]) + v, w = Vertex([0, 0]), Vertex([1, 1]) with self.assertRaises(ValueError): self.g.add_edge(MyEdge(v, w)) diff --git a/tests/test_add_vertex_names.py b/tests/test_add_vertex_names.py index 59d4c07d..c500785f 100644 --- a/tests/test_add_vertex_names.py +++ b/tests/test_add_vertex_names.py @@ -8,7 +8,7 @@ import unittest -from pgraph import DGraph, UGraph, UVertex +from pgraph import DGraph, UGraph, Vertex class _AddVertex: @@ -103,13 +103,13 @@ def test_wrong_dimension_changes_nothing(self): class TestUGraph(_AddVertex, unittest.TestCase): - Graph, Vertex = UGraph, UVertex + Graph = UGraph + Vertex = Vertex class TestDGraph(_AddVertex, unittest.TestCase): - from pgraph import DVertex as Vertex - Graph = DGraph + Vertex = Vertex if __name__ == "__main__": diff --git a/tests/test_adjacent.py b/tests/test_adjacent.py index 3dd159d5..24ecbf3a 100644 --- a/tests/test_adjacent.py +++ b/tests/test_adjacent.py @@ -9,18 +9,16 @@ import unittest import warnings -from pgraph import DGraph, UGraph, DVertex, UVertex -from pgraph.PGraph import Vertex +from pgraph import DGraph, UGraph, Vertex class TestAliases(unittest.TestCase): def test_aliases_are_the_same_function(self): - for cls in (Vertex, UVertex, DVertex): - with self.subTest(cls=cls.__name__): - self.assertIs(cls.neighbours, cls.adjacent) - self.assertIs(cls.neighbors, cls.adjacent) - self.assertIs(cls.isneighbour, cls.isadjacent) - self.assertIs(cls.isneighbor, cls.isadjacent) + cls = Vertex + self.assertIs(cls.neighbours, cls.adjacent) + self.assertIs(cls.neighbors, cls.adjacent) + self.assertIs(cls.isneighbour, cls.isadjacent) + self.assertIs(cls.isneighbor, cls.isadjacent) def test_no_warnings(self): g = UGraph() diff --git a/tests/test_attribute_hygiene.py b/tests/test_attribute_hygiene.py index ce58e7b8..be3ce994 100644 --- a/tests/test_attribute_hygiene.py +++ b/tests/test_attribute_hygiene.py @@ -13,7 +13,7 @@ import unittest -from pgraph import Graph, Vertex, DGraph, UGraph, UVertex, exporters +from pgraph import Graph, Vertex, DGraph, UGraph, exporters class TestLabelIsReadOnly(unittest.TestCase): @@ -34,7 +34,7 @@ def test_still_computed(self): self.assertEqual({v.label for v in self.g}, {0, 1}) def test_none_until_the_components_are_found(self): - self.assertIsNone(UVertex(name="loose").label) + self.assertIsNone(Vertex(name="loose").label) def test_directed_graph_too(self): g = DGraph() @@ -82,7 +82,7 @@ def test_cannot_clear_the_name_of_a_vertex_in_a_graph(self): self.assertEqual(self.a.name, "a") def test_before_adding_is_just_assignment(self): - v = UVertex([0, 0]) + v = Vertex([0, 0]) v.name = "free" v.name = None v.name = "again" @@ -123,8 +123,9 @@ def test_basegraph_is_exported(self): self.assertTrue(issubclass(DGraph, Graph)) self.assertIsInstance(UGraph(), Graph) - def test_basevertex_is_exported(self): - self.assertTrue(issubclass(UVertex, Vertex)) + def test_vertex_is_exported(self): + self.assertIs(UGraph().add_vertex([0, 0]).__class__, Vertex) + self.assertIs(DGraph().add_vertex([0, 0]).__class__, Vertex) def test_the_unreleased_private_name_is_gone(self): import pgraph.PGraph as module diff --git a/tests/test_detach_hook.py b/tests/test_detach_hook.py index 594eb76e..f7e6c0d5 100644 --- a/tests/test_detach_hook.py +++ b/tests/test_detach_hook.py @@ -1,43 +1,43 @@ """ ``_attach`` records an edge in the vertices' edge lists when it is connected; ``_detach`` is its twin, undoing exactly that when the edge is removed, so each -vertex kind states how an edge enters and leaves its lists once, and +graph kind states how an edge enters and leaves its vertices' lists once, and ``remove_edge`` / ``remove_vertex`` need no knowledge of which kind they hold. """ import unittest -from pgraph import DGraph, DVertex, Edge, UGraph, UVertex -from pgraph.PGraph import Vertex +from pgraph import DGraph, Edge, Graph, UGraph, Vertex class TestHookIsDefinedPerKind(unittest.TestCase): def test_kinds_supply_it(self): - self.assertIsNot(UVertex._detach, Vertex._detach) - self.assertIsNot(DVertex._detach, Vertex._detach) - self.assertIsNot(UVertex._detach, DVertex._detach) + self.assertIsNot(UGraph._detach, Graph._detach) + self.assertIsNot(DGraph._detach, Graph._detach) + self.assertIsNot(UGraph._detach, DGraph._detach) def test_base_hook_is_not_implemented(self): - a, b = Vertex(name="a"), Vertex(name="b") + g = UGraph() + a, b = g.add_vertex(name="a"), g.add_vertex(name="b") with self.assertRaises(NotImplementedError): - a._detach(Edge(a, b), b) + Graph._detach(g, Edge(a, b)) class _Detach: Graph: type - Vertex: type def setUp(self): self.calls = [] outer = self - class Recording(self.Vertex): - def _detach(self, edge, dest): + class Recording(self.Graph): + def _detach(self, edge): # at this point the edge still knows its endpoints - outer.calls.append((self, edge, dest, edge.v1, edge.v2)) - super()._detach(edge, dest) + outer.calls.append((edge, edge.v1, edge.v2)) + super()._detach(edge) - self.g = self.Graph(vertex_class=Recording) + self.Recording = Recording + self.g = Recording() self.a = self.g.add_vertex([0, 0], name="a") self.b = self.g.add_vertex([1, 0], name="b") self.c = self.g.add_vertex([2, 0], name="c") @@ -53,10 +53,8 @@ def test_remove_edge_calls_the_hook_once(self): e = self.a.connect(self.b) self.g.remove_edge(e) self.assertEqual(len(self.calls), 1) - v, edge, dest, v1, v2 = self.calls[0] - self.assertIs(v, self.a) + edge, v1, v2 = self.calls[0] self.assertIs(edge, e) - self.assertIs(dest, self.b) self.assertEqual((v1, v2), (self.a, self.b)) self.assertEqual(self.g.ne, 0) self.assertIsNone(e.v1) @@ -94,12 +92,12 @@ def test_removed_vertex_no_longer_points_at_the_graph(self): with self.assertRaises(ValueError): self.a.remove() # not in a graph any more, a clear error # and it can join another graph - self.Graph(vertex_class=type(self.a)).add_vertex(self.a) + self.Graph().add_vertex(self.a) self.assertEqual(self.g.n, 2) class TestUndirectedDetach(_Detach, unittest.TestCase): - Graph, Vertex = UGraph, UVertex + Graph = UGraph def test_both_ends_are_cleared(self): e = self.a.connect(self.b) @@ -114,7 +112,7 @@ def test_self_loop_is_cleared_from_both_entries(self): class TestDirectedDetach(_Detach, unittest.TestCase): - Graph, Vertex = DGraph, DVertex + Graph = DGraph def test_only_the_source_held_it(self): e = self.a.connect(self.b) diff --git a/tests/test_graph.py b/tests/test_graph.py index 21e7eae5..31e36280 100644 --- a/tests/test_graph.py +++ b/tests/test_graph.py @@ -14,15 +14,15 @@ def test_constructor(self): v1 = g.add_vertex() v2 = g.add_vertex() self.assertEqual(g.n, 2) - self.assertIsInstance(v1, UVertex) - self.assertIsInstance(v2, UVertex) + self.assertIsInstance(v1, Vertex) + self.assertIsInstance(v2, Vertex) def test_constructor2(self): g = UGraph() v = g.add_vertex([1,2,3]) - self.assertIsInstance(v, UVertex) + self.assertIsInstance(v, Vertex) self.assertEqual(v.x, 1) self.assertEqual(v.y, 2) self.assertEqual(v.z, 3) @@ -74,7 +74,7 @@ def test_constructor3(self): self.assertIs(g[0], v1) self.assertIs(g[0], v1) - class MyNode(UVertex): + class MyNode(Vertex): def __init__(self, a): super().__init__() self.a = a @@ -339,7 +339,7 @@ def test_add_vertex(self): g = UGraph() - v = UVertex() + v = Vertex() g.add_vertex(v) self.assertEqual(v.name, '#0') @@ -411,10 +411,10 @@ def test_contains(self): def test_Dict(self): - v1 = UVertex(name='v1') - v2 = UVertex(name='v2') - v3 = UVertex(name='v3') - v4 = UVertex(name='v4') + v1 = Vertex(name='v1') + v2 = Vertex(name='v2') + v3 = Vertex(name='v3') + v4 = Vertex(name='v4') parent = {} parent[v2] = v1 @@ -829,18 +829,18 @@ def test_constructor(self): v1 = g.add_vertex() v2 = g.add_vertex() self.assertEqual(g.n, 2) - self.assertIsInstance(v1, DVertex) - self.assertIsInstance(v2, DVertex) + self.assertIsInstance(v1, Vertex) + self.assertIsInstance(v2, Vertex) def test_Dict(self): - # regression test: Dict() used to hardcode UVertex() regardless of + # regression test: Dict() used to hardcode Vertex() regardless of # the graph type, silently producing wrong-typed vertices in a DGraph g = DGraph.Dict({'child': 'parent'}) self.assertIsInstance(g, DGraph) self.assertEqual(g.n, 2) for v in g: - self.assertIsInstance(v, DVertex) + self.assertIsInstance(v, Vertex) def test_iscyclic(self): @@ -926,7 +926,7 @@ def test_matrices(self): def test_remove_edge(self): # regression test: removing a directed edge used to always crash, # since it assumed both endpoints track the edge in their own - # _edgelist, but a DVertex only tracks outgoing edges + # _edgelist, but a Vertex only tracks outgoing edges g = DGraph() v1 = g.add_vertex(name='1') v2 = g.add_vertex(name='2') diff --git a/tests/test_graph_classes.py b/tests/test_graph_classes.py index ba36c665..838cd6e6 100644 --- a/tests/test_graph_classes.py +++ b/tests/test_graph_classes.py @@ -11,7 +11,7 @@ import numpy as np -from pgraph import DGraph, DVertex, Edge, UGraph, UVertex +from pgraph import DGraph, Edge, UGraph, Vertex class MyEdge(Edge): @@ -20,10 +20,14 @@ def __init__(self, *args, **kwargs): self.traversals = 0 +class OtherVertex(Vertex): + """A vertex class that is not the one a graph under test is configured for""" + + class _Classes: Graph: type - Vertex: type # UVertex or DVertex - OtherVertex: type # the wrong kind for this graph + Vertex: type + OtherVertex: type def setUp(self): class MyVertex(self.Vertex): @@ -80,14 +84,15 @@ def test_add_vertex_accepts_instances_of_vertex_class(self): self.assertIs(self.g.add_vertex(v), v) def test_add_vertex_rejects_plain_vertex_of_right_kind(self): - # the graph was configured to hold MyVertex, not just any UVertex/DVertex + # the graph was configured to hold MyVertex, not just any Vertex with self.assertRaises(TypeError): self.g.add_vertex(self.Vertex([5, 5])) self.assertEqual(self.g.n, 0) - def test_add_vertex_rejects_wrong_kind(self): + def test_add_vertex_rejects_a_vertex_of_another_class(self): with self.assertRaises(TypeError): self.g.add_vertex(self.OtherVertex([5, 5])) + self.assertEqual(self.g.n, 0) def test_connect_rejects_edge_of_wrong_class(self): a = self.g.add_vertex([0, 0]) @@ -106,13 +111,13 @@ def test_default_graph_still_accepts_subclasses(self): self.assertIsInstance(a.connect(b, edge=MyEdge(a, b)), MyEdge) def test_bad_vertex_class(self): - for bad in (self.OtherVertex, int, "UVertex", self.Vertex([0, 0]), Edge): + for bad in (int, "Vertex", self.Vertex([0, 0]), Edge): with self.subTest(bad=bad): with self.assertRaises(TypeError): self.Graph(vertex_class=bad) def test_bad_edge_class(self): - for bad in (UVertex, int, "Edge", Edge(cost=1)): + for bad in (Vertex, int, "Edge", Edge(cost=1)): with self.subTest(bad=bad): with self.assertRaises(TypeError): self.Graph(edge_class=bad) @@ -157,14 +162,14 @@ def test_planning_unaffected(self): class TestUGraphClasses(_Classes, unittest.TestCase): Graph = UGraph - Vertex = UVertex - OtherVertex = DVertex + Vertex = Vertex + OtherVertex = OtherVertex class TestDGraphClasses(_Classes, unittest.TestCase): Graph = DGraph - Vertex = DVertex - OtherVertex = UVertex + Vertex = Vertex + OtherVertex = OtherVertex if __name__ == "__main__": diff --git a/tests/test_pickle.py b/tests/test_pickle.py index 17955723..c2b9fad6 100644 --- a/tests/test_pickle.py +++ b/tests/test_pickle.py @@ -12,7 +12,7 @@ import numpy as np -from pgraph import DGraph, DVertex, Edge, UGraph, UVertex +from pgraph import DGraph, Vertex, Edge, UGraph def dumps(obj, protocol=None): @@ -27,18 +27,12 @@ def manhattan(v): return float(np.abs(v).sum()) -class TaggedVertex(UVertex): +class TaggedVertex(Vertex): def __init__(self, tag, coord=None, name=None): # a different signature super().__init__(coord=coord, name=name) self.tag = tag -class TaggedDVertex(DVertex): - def __init__(self, tag, coord=None, name=None): - super().__init__(coord=coord, name=name) - self.tag = tag - - class WeightedEdge(Edge): def __init__(self, v1=None, v2=None, cost=None, data=None): super().__init__(v1, v2, cost=cost, data=data) @@ -239,7 +233,8 @@ def test_in_edge_order_survives(self): class TestSubclasses(unittest.TestCase): def test_vertex_and_edge_classes(self): - for gcls, vcls in ((UGraph, TaggedVertex), (DGraph, TaggedDVertex)): + vcls = TaggedVertex + for gcls in (UGraph, DGraph): with self.subTest(graph=gcls.__name__): g = gcls(vertex_class=vcls, edge_class=WeightedEdge) a = g.add_vertex([0, 0], name="a") diff --git a/tests/test_unified_graph.py b/tests/test_unified_graph.py new file mode 100644 index 00000000..b85de3cf --- /dev/null +++ b/tests/test_unified_graph.py @@ -0,0 +1,210 @@ +"""One vertex class, and a graph that is directed or not. + +A vertex has no direction of its own: what differs between an undirected and a +directed graph is how an edge is recorded at its ends, which is the graph's +decision. So there is one ``Vertex``; ``Graph(directed=...)`` chooses the kind; +``UGraph`` and ``DGraph`` remain the classes; and ``UVertex`` / ``DVertex`` are +deprecated names for ``Vertex``. +""" + +import copy +import pickle # nosec B403 nosemgrep - the tests pickle their own graphs +import unittest +import warnings + +import numpy as np + +import pgraph +from pgraph import DGraph, Edge, Graph, UGraph, Vertex + + +class TestGraphChoosesTheKind(unittest.TestCase): + def test_default_is_undirected(self): + g = Graph() + self.assertIsInstance(g, UGraph) + self.assertFalse(g.directed) + + def test_directed_true_is_a_dgraph(self): + g = Graph(directed=True) + self.assertIsInstance(g, DGraph) + self.assertTrue(g.directed) + + def test_directed_false_is_a_ugraph(self): + self.assertIsInstance(Graph(directed=False), UGraph) + + def test_the_kind_can_be_a_variable(self): + for flag in (False, True): + with self.subTest(directed=flag): + self.assertEqual(Graph(directed=flag).directed, flag) + + def test_other_arguments_pass_through(self): + g = Graph(directed=True, metric="L1", dim=3) + self.assertEqual(g.metric(np.r_[1.0, -2.0, 3.0]), 6.0) + with self.assertRaises(ValueError): + g.add_vertex([1, 2]) + + def test_graph_arguments_are_keyword_only(self): + with self.assertRaises(TypeError): + Graph(6) + with self.assertRaises(TypeError): + Graph(True) + + def test_the_classes_report_their_kind(self): + self.assertFalse(UGraph().directed) + self.assertTrue(DGraph().directed) + + def test_directed_may_repeat_the_class(self): + self.assertTrue(DGraph(directed=True).directed) + self.assertFalse(UGraph(directed=False).directed) + + def test_directed_may_not_contradict_the_class(self): + with self.assertRaises(ValueError): + UGraph(directed=True) + with self.assertRaises(ValueError): + DGraph(directed=False) + + def test_directed_is_read_only(self): + g = Graph() + with self.assertRaises(AttributeError): + g.directed = True + + +class TestOneVertexClass(unittest.TestCase): + def test_both_kinds_build_the_same_class(self): + self.assertIs(type(UGraph().add_vertex([0, 0])), Vertex) + self.assertIs(type(DGraph().add_vertex([0, 0])), Vertex) + + def test_a_vertex_joins_either_kind(self): + for kind in (UGraph, DGraph): + with self.subTest(kind=kind.__name__): + g = kind() + v = Vertex([1, 2], name="v") + self.assertIs(g.add_vertex(v), v) + self.assertIs(v._graph, g) + + def test_a_removed_vertex_may_join_the_other_kind(self): + u, d = UGraph(), DGraph() + v = u.add_vertex([0, 0], name="v") + u.remove_vertex(v) + self.assertIs(d.add_vertex(v), v) + + def test_user_subclass_serves_both_kinds(self): + class Mine(Vertex): + pass + + for kind in (UGraph, DGraph): + with self.subTest(kind=kind.__name__): + g = kind(vertex_class=Mine) + self.assertIs(type(g.add_vertex([0, 0])), Mine) + + def test_vertex_class_must_be_a_vertex(self): + for bad in (int, "Vertex", Edge, Vertex([0, 0])): + with self.subTest(bad=bad): + with self.assertRaises(TypeError): + Graph(vertex_class=bad) + + def test_the_kind_is_the_graphs_decision(self): + # the same two vertices, joined in a graph of each kind + for kind in (UGraph, DGraph): + g = kind() + a, b = g.add_vertex(name="a"), g.add_vertex(name="b") + e = a.connect(b) + self.assertEqual(a.edges(), [e]) + self.assertEqual(b.edges(), [e] if not g.directed else []) + self.assertEqual(b.in_edges(), [e]) + self.assertEqual(a.in_edges(), [] if g.directed else [e]) + + +class TestDeprecatedNames(unittest.TestCase): + def test_the_old_names_are_vertex(self): + for name in ("UVertex", "DVertex"): + with self.subTest(name=name): + with self.assertWarns(DeprecationWarning) as cm: + cls = getattr(pgraph, name) + self.assertIs(cls, Vertex) + self.assertIn("Vertex", str(cm.warning)) + + def test_importing_warns_and_works(self): + with self.assertWarns(DeprecationWarning): + from pgraph import UVertex + + self.assertIs(UVertex, Vertex) + with self.assertWarns(DeprecationWarning): + from pgraph import DVertex + + self.assertIs(DVertex, Vertex) + + def test_subclassing_an_old_name_still_works(self): + with warnings.catch_warnings(): + warnings.simplefilter("ignore", DeprecationWarning) + + class Mine(pgraph.UVertex): + def __init__(self, coord=None, name=None): + super().__init__(coord, name=name) + self.extra = 1 + + for kind in (UGraph, DGraph): + g = kind(vertex_class=Mine) + self.assertEqual(g.add_vertex([0, 0]).extra, 1) + + def test_other_missing_names_are_still_errors(self): + with self.assertRaises(AttributeError): + pgraph.NoSuchThing + + def test_the_old_names_are_not_advertised(self): + self.assertNotIn("UVertex", pgraph.__all__) + self.assertNotIn("DVertex", pgraph.__all__) + + def test_what_is_advertised_does_not_warn(self): + # this is what ``from pgraph import *`` fetches + with warnings.catch_warnings(): + warnings.simplefilter("error") + for name in pgraph.__all__: + getattr(pgraph, name) + + +class TestConstructorsAreKindAware(unittest.TestCase): + def test_adjacency_on_graph(self): + A = np.array([[0, 1, 0], [0, 0, 1], [1, 0, 0]]) + u = Graph.Adjacency(A) + d = Graph.Adjacency(A, directed=True) + self.assertIsInstance(u, UGraph) + self.assertIsInstance(d, DGraph) + self.assertEqual(d.ne, 3) + self.assertEqual(u.ne, 2) # only the upper triangle is used + np.testing.assert_array_equal(d.adjacency(), A) + + def test_dict_on_graph(self): + d = {"b": "a", "c": "a"} + h = Graph.Dict(d, directed=True) + self.assertIsInstance(h, DGraph) + self.assertEqual(h.ne, 2) + self.assertEqual([v.name for v in h["a"].successors()], ["b", "c"]) + self.assertIsInstance(Graph.Dict(d), UGraph) + + def test_copy_and_pickle_keep_the_kind(self): + for flag in (False, True): + with self.subTest(directed=flag): + g = Graph(directed=flag) + a, b = g.add_vertex(name="a"), g.add_vertex(name="b") + g.add_edge(a, b) + for h in (g.copy(), copy.deepcopy(g), pickle.loads(pickle.dumps(g))): # nosec nosemgrep + self.assertEqual(h.directed, flag) + self.assertEqual(type(h), type(g)) + self.assertEqual(h.validate(), []) + + +class TestEdgeArrow(unittest.TestCase): + def test_arrow_follows_the_graph(self): + for flag, arrow in ((False, "--"), (True, "->")): + g = Graph(directed=flag) + a, b = g.add_vertex(name="a"), g.add_vertex(name="b") + self.assertIn(arrow, str(g.add_edge(a, b))) + + def test_a_standalone_edge_has_no_arrow_to_show(self): + e = Edge(Vertex(name="a"), Vertex(name="b"), cost=1) + self.assertIn("--", str(e)) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_validate.py b/tests/test_validate.py index 62392827..6d714834 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -12,7 +12,7 @@ import random import unittest -from pgraph import DGraph, Edge, UGraph, UVertex +from pgraph import DGraph, Edge, UGraph, Vertex def poison(edge, name, value): @@ -75,7 +75,7 @@ def test_random_edit_sequences(self): self.assertEqual(g.validate(), []) def test_user_vertex_and_edge_classes(self): - class V(UVertex): + class V(Vertex): pass class E(Edge): @@ -151,12 +151,12 @@ def test_vertex_missing_from_the_name_table(self): self.assertIn("'d'", self.problems(g)) def test_vertex_of_the_wrong_class(self): - class V(UVertex): + class V(Vertex): pass g = UGraph(vertex_class=V) g.add_vertex([0, 0], name="a") - g._vertexlist.append(UVertex([1, 0], name="plain")) + g._vertexlist.append(Vertex([1, 0], name="plain")) g._vertexlist[-1]._graph = g g._vertexdict["plain"] = g._vertexlist[-1] self.assertIn("plain", self.problems(g)) diff --git a/tests/test_vertex_hook.py b/tests/test_vertex_hook.py index ee574739..e7c468e6 100644 --- a/tests/test_vertex_hook.py +++ b/tests/test_vertex_hook.py @@ -1,79 +1,82 @@ """ -``Vertex.connect()`` is the one public method; what differs between an -undirected and a directed vertex is a single bookkeeping hook, ``_attach``, -that ``connect()`` calls once the connection has been validated and the edge -registered with the graph. +``Vertex.connect()`` is the one public method. A vertex has no direction of +its own, so what differs between an undirected and a directed graph is a single +bookkeeping hook of the *graph*, ``_attach``, that ``connect()`` calls once the +connection has been validated and the edge registered with the graph. -This pins the design: ``connect`` is defined once, the hook is what a vertex -kind supplies, and nothing is attached when validation fails. +This pins the design: ``connect`` is defined once, on the vertex, the hook is +what a graph kind supplies, and nothing is attached when validation fails. """ import unittest -from pgraph import DGraph, Edge, UGraph, DVertex, UVertex -from pgraph.PGraph import Vertex +from pgraph import DGraph, Edge, Graph, UGraph, Vertex class TestConnectIsDefinedOnce(unittest.TestCase): - def test_subclasses_inherit_connect(self): - self.assertIs(UVertex.connect, Vertex.connect) - self.assertIs(DVertex.connect, Vertex.connect) + def test_there_is_one_vertex_class_and_it_has_no_hook(self): + self.assertFalse(hasattr(Vertex, "_attach")) + self.assertFalse(hasattr(Vertex, "_detach")) - def test_subclasses_supply_the_hook(self): - self.assertIsNot(UVertex._attach, Vertex._attach) - self.assertIsNot(DVertex._attach, Vertex._attach) - self.assertIsNot(UVertex._attach, DVertex._attach) + def test_graph_kinds_supply_the_hook(self): + self.assertIsNot(UGraph._attach, Graph._attach) + self.assertIsNot(DGraph._attach, Graph._attach) + self.assertIsNot(UGraph._attach, DGraph._attach) def test_base_hook_is_not_implemented(self): - a, b = Vertex(name="a"), Vertex(name="b") + g = UGraph() + a, b = g.add_vertex(name="a"), g.add_vertex(name="b") with self.assertRaises(NotImplementedError): - a._attach(Edge(a, b), b) + Graph._attach(g, Edge(a, b)) -class TestBareVertex(unittest.TestCase): - """Vertex stays constructible (see policy.rst) but can never join a graph""" +class TestStandaloneVertex(unittest.TestCase): + """A vertex is constructible alone (see policy.rst) and joins either kind""" def test_instantiable(self): v = Vertex([1, 2], name="v") self.assertEqual(v.name, "v") - def test_cannot_connect_or_be_added(self): + def test_cannot_connect_until_added(self): a, b = Vertex(name="a"), Vertex(name="b") with self.assertRaises(ValueError): a.connect(b) - with self.assertRaises(TypeError): - UGraph().add_vertex(a) - with self.assertRaises(TypeError): - DGraph().add_vertex(a) + + def test_can_be_added_to_either_kind(self): + for kind in (UGraph, DGraph): + with self.subTest(kind=kind.__name__): + g = kind() + a, b = Vertex(name="a"), Vertex(name="b") + g.add_vertex(a) + g.add_vertex(b) + self.assertIsInstance(a.connect(b), Edge) + self.assertEqual(g.ne, 1) class _Hook: Graph: type - Vertex: type def setUp(self): self.calls = [] outer = self - class Recording(self.Vertex): - def _attach(self, edge, dest): + class Recording(self.Graph): + def _attach(self, edge): # the edge is already registered with the graph at this point - outer.calls.append( - (self, edge, dest, edge in self._graph._edges) - ) - super()._attach(edge, dest) + outer.calls.append((edge, edge in self._edges)) + super()._attach(edge) - self.g = self.Graph(vertex_class=Recording) + self.g = Recording() self.a = self.g.add_vertex([0, 0], name="a") self.b = self.g.add_vertex([1, 0], name="b") def test_attach_called_once_with_the_edge(self): e = self.a.connect(self.b, cost=3) self.assertEqual(len(self.calls), 1) - v, edge, dest, registered = self.calls[0] - self.assertIs(v, self.a) + edge, registered = self.calls[0] self.assertIs(edge, e) - self.assertIs(dest, self.b) + self.assertIs(edge.v1, self.a) + self.assertIs(edge.v2, self.b) self.assertTrue(registered) def test_returns_the_edge(self): @@ -88,7 +91,7 @@ def test_nothing_attached_when_validation_fails(self): self.a.connect(other) # different graph with self.assertRaises(ValueError): self.a.connect(self.b, edge=Edge(self.b, self.b)) # wrong endpoints - ungraphed = self.Vertex([9, 9]) + ungraphed = Vertex([9, 9]) with self.assertRaises(ValueError): self.a.connect(ungraphed) self.assertEqual(self.calls, []) @@ -102,7 +105,7 @@ def test_add_edge_and_edge_connect_go_through_the_hook(self): class TestUndirectedHook(_Hook, unittest.TestCase): - Graph, Vertex = UGraph, UVertex + Graph = UGraph def test_edge_is_held_by_both_ends(self): e = self.a.connect(self.b) @@ -116,7 +119,7 @@ def test_self_loop_is_held_twice_by_its_vertex(self): class TestDirectedHook(_Hook, unittest.TestCase): - Graph, Vertex = DGraph, DVertex + Graph = DGraph def test_edge_is_held_by_the_source_only(self): e = self.a.connect(self.b) @@ -132,13 +135,13 @@ def test_self_loop_is_held_once(self): class TestHookIsTheExtensionPoint(unittest.TestCase): def test_custom_bookkeeping(self): - # a user kind that records the edge on the source only, though it is - # built on the undirected vertex: only the hook has to change - class OneSided(UVertex): - def _attach(self, edge, dest): - self._edgelist.append(edge) + # a user graph that records the edge on the start vertex only, though + # it is built on the undirected graph: only the hook has to change + class OneSided(UGraph): + def _attach(self, edge): + edge.v1._edgelist.append(edge) - g = UGraph(vertex_class=OneSided) + g = OneSided() a = g.add_vertex([0, 0], name="a") b = g.add_vertex([1, 0], name="b") e = a.connect(b) From bec2ded2f66852255a950b0d822b3c0f55db89b1 Mon Sep 17 00:00:00 2001 From: Peter Corke Date: Mon, 12 Oct 2026 10:01:12 +1000 Subject: [PATCH 2/3] docs: follow the single Vertex class The examples, the policy page and the README use Vertex, the pages for UVertex and DVertex are removed, and the common page says that a vertex has no direction of its own. Only what the new API makes wrong or runnable-with-a-warning is changed; the full documentation and diagram pass follows. Co-Authored-By: Claude Sonnet 5.5 --- README.md | 6 +++--- docs/source/common.rst | 20 +++++++---------- docs/source/edges_vertices.rst | 12 +++++------ docs/source/intro.rst | 8 +++---- docs/source/modification.rst | 8 +++---- docs/source/policy.rst | 39 +++++++++++++++++----------------- 6 files changed, 43 insertions(+), 50 deletions(-) diff --git a/README.md b/README.md index 20b17ca1..a6545fe7 100644 --- a/README.md +++ b/README.md @@ -96,7 +96,7 @@ Graphs belong to the class `UGraph` or `DGraph` for undirected or directed graph - `g.incidence()` incidence matrix ### Properties and methods of a vertex -Vertices belong to the class `UVertex` (for undirected graphs) or `DVertex` (for directed graphs), which are each subclasses of `Vertex`. +Vertices are instances of the class `Vertex`, for both undirected and directed graphs. - `v.coord` the coordinate vector for embedded graph (optional) - `v.name` the name of the vertex (optional) @@ -123,11 +123,11 @@ Each edge references a pair of vertices, the graph lists all of its edges in `g. Consider a user class `Foo` that we would like to connect using a graph _overlay_, ie. instances of `Foo` becomes vertices in a graph. -- Have it subclass either `DVertex` or `UVertex` depending on graph type +- Have it subclass `Vertex` - Then place instances of `Foo` into the graph using `add_vertex` and create edges as required ``` -class Foo(UVertex): +class Foo(Vertex): # foo stuff goes here f1 = Foo(...) diff --git a/docs/source/common.rst b/docs/source/common.rst index 95c444ee..5ad0a211 100644 --- a/docs/source/common.rst +++ b/docs/source/common.rst @@ -11,18 +11,14 @@ The graph classes represent a single graph, either directed or undirected, and t :private-bases: :top-classes: pgraph.PGraph.Graph -The vertices in a directed graph (``DGraph``) inherit from :class:`DVertex` and the -vertices in an undirected graph (``UGraph``) inherit from :class:`UVertex`. - -.. inheritance-diagram:: pgraph.PGraph.UVertex pgraph.PGraph.DVertex - :parts: 1 - -The vertex classes share -almost their entire interface, defined once on their common base classes -(:class:`Graph`, :class:`Vertex`). This section documents that -shared interface by task rather than by class; see :doc:`construction` for -the :class:`DGraph` and :class:`UGraph` classes and :doc:`edges_vertices` for -what's specific to directed vs. undirected vertices. +The vertices of both kinds of graph are :class:`Vertex` objects: a vertex has +no direction of its own, the graph it belongs to decides how an edge is +recorded at its ends. + +The graph classes share almost their entire interface, defined once on their +common base class :class:`Graph`. This section documents that shared interface +by task rather than by class; see :doc:`construction` for the :class:`DGraph` +and :class:`UGraph` classes and :doc:`edges_vertices` for vertices and edges. Edges are represented by the :class:`Edge` class, which is used by both directed and undirected graphs. diff --git a/docs/source/edges_vertices.rst b/docs/source/edges_vertices.rst index d0d6ea5c..d4161126 100644 --- a/docs/source/edges_vertices.rst +++ b/docs/source/edges_vertices.rst @@ -22,7 +22,7 @@ and to add them to the graph. .. runblock:: pycon :session: - >>> from pgraph import UGraph, UVertex, Edge + >>> from pgraph import UGraph, Vertex, Edge >>> import numpy as np >>> g = UGraph() @@ -31,8 +31,8 @@ Next, we will explicitly create two vertices .. runblock:: pycon :session: - >>> v1 = UVertex(np.random.rand(2)) - >>> v2 = UVertex(np.random.rand(2)) + >>> v1 = Vertex(np.random.rand(2)) + >>> v2 = Vertex(np.random.rand(2)) and then add them to the graph @@ -77,7 +77,7 @@ Alternatively, we can create a vertex and *connect* it to an existing vertex, wh .. runblock:: pycon :session: - >>> v3 = UVertex(np.random.rand(2)) + >>> v3 = Vertex(np.random.rand(2)) >>> g.add_vertex(v3) >>> v2.connect(v3) >>> print(g) @@ -88,11 +88,9 @@ The ``connect`` method is available on both the directed and undirected vertex classes. -.. note:: Where ever a vertex or edge is required, a subclass of the appropriate base class can be used. For example, we can create a subclass of ``UVertex`` called ``MyVertex`` and a subclass of ``Edge`` called ``MyEdge`` and then create a new graph using these classes. +.. note:: Where ever a vertex or edge is required, a subclass of the appropriate base class can be used. For example, we can create a subclass of ``Vertex`` called ``MyVertex`` and a subclass of ``Edge`` called ``MyEdge`` and then create a new graph using these classes. .. toctree:: :maxdepth: 2 - dvertex - uvertex edge diff --git a/docs/source/intro.rst b/docs/source/intro.rst index 6c57ccb6..2b78d288 100644 --- a/docs/source/intro.rst +++ b/docs/source/intro.rst @@ -60,7 +60,7 @@ we can obtain the reference by indexing the graph object. For example vertex 1 i >>> g[1] -and is an instance of ``UVertex``, a vertex in an undirected graph; it has the coordinate +and is an instance of ``Vertex``; it has the coordinate :math:`(0.6028, 0.5449)` and has been assigned a default name ``"#1"`` but we could specify an explicit name to ``add_vertex``. We can also reference a vertex by its name @@ -201,15 +201,15 @@ Arbitrary data can be attached to any vertex or edge by assigning values to attr This exploits the flexible nature of Python, but care must be taken to avoid name clashes with existing attributes. -A more robust and Pythonic approach is to create a subclass of the ``UVertex`` or ``DVertex`` classes class and -declare them at graph creation time. For example, we can create a subclass of ``UVertex`` called ``MyVertex`` and a subclass of ``Edge`` called ``MyEdge`` and then create a new graph using these classes. +A more robust and Pythonic approach is to create a subclass of the ``Vertex`` class and +declare them at graph creation time. For example, we can create a subclass of ``Vertex`` called ``MyVertex`` and a subclass of ``Edge`` called ``MyEdge`` and then create a new graph using these classes. .. runblock:: pycon :session: >>> import pgraph >>> import numpy as np - >>> class MyVertex(pgraph.UVertex): + >>> class MyVertex(pgraph.Vertex): ... pass >>> class MyEdge(pgraph.Edge): ... pass diff --git a/docs/source/modification.rst b/docs/source/modification.rst index 96a69609..cc548c8b 100644 --- a/docs/source/modification.rst +++ b/docs/source/modification.rst @@ -1,10 +1,10 @@ Adding and removing vertices and edges ======================================== -Building up and tearing down a graph's structure. ``connect()`` is shown -here in its base-class form; :doc:`DVertex ` and -:doc:`UVertex ` override it with different edge-list bookkeeping --- see those pages for the concrete behaviour. See :class:`Edge` for +Building up and tearing down a graph's structure. ``connect()`` is the same +for both kinds of graph: the graph decides how the edge is recorded at its +vertices, so an undirected edge is found from either end and a directed one +leaves its start vertex and enters its end vertex. See :class:`Edge` for constructing/removing an edge directly rather than via a vertex or graph. Graph diff --git a/docs/source/policy.rst b/docs/source/policy.rst index 463dc142..6e3c35cc 100644 --- a/docs/source/policy.rst +++ b/docs/source/policy.rst @@ -1,7 +1,7 @@ Object lifecycle and graph-membership policy ============================================= -``Vertex``, ``UVertex``, ``DVertex`` and ``Edge`` can all be constructed +``Vertex`` and ``Edge`` can both be constructed directly, independent of any graph. This is deliberate -- it is not an oversight and not a "factory-only" design -- but it means the rules for what those objects can and can't do *before* they belong to a graph need to be @@ -10,7 +10,7 @@ stated explicitly rather than left implicit. Standalone construction ------------------------ -A vertex or edge built directly, e.g. ``UVertex(coord=[1, 2], name="A")`` or +A vertex or edge built directly, e.g. ``Vertex(coord=[1, 2], name="A")`` or ``Edge(v1, v2, cost=5.0)``, is a fully valid object in its own right. The main supported use of this is lightweight, graph-independent work: building a couple of vertices purely to demonstrate or test :class:`Edge` cost @@ -23,17 +23,17 @@ anything (see below) until it is added to a graph via Adding a vertex to a graph --------------------------- -:meth:`UGraph.add_vertex` and :meth:`DGraph.add_vertex` each only accept -their own matching vertex subclass -- a ``UVertex`` (or coordinate data, from -which one is built) for :class:`UGraph`, a ``DVertex`` for :class:`DGraph`. -Passing the wrong kind of vertex -- a plain ``Vertex``, or the other -graph's subclass -- raises ``TypeError`` rather than being silently accepted -or converted. +A vertex is the same for undirected and directed graphs: it has no +direction of its own, and the graph it belongs to decides how an edge is +recorded at its ends. :meth:`Graph.add_vertex` accepts coordinate data, from +which a vertex is built, or an existing ``Vertex`` that does not belong to a +graph. A graph given a ``vertex_class`` only accepts instances of that class; +passing any other vertex raises ``TypeError`` rather than being silently +accepted or converted. A vertex that already belongs to a graph raises +``ValueError``; remove it from that graph first. This is the mechanism that makes graph membership meaningful: once added, a -vertex's ``._graph`` is fixed to that specific graph instance, and its -concrete type (``UVertex`` or ``DVertex``) is guaranteed correct for that -graph's edge semantics (undirected vs. directed edge-list bookkeeping). +vertex's ``._graph`` is fixed to that specific graph instance. Connecting two vertices ------------------------- @@ -46,12 +46,9 @@ Connecting two vertices - If the two vertices belong to different graph instances, ``connect()`` raises ``ValueError``. -There is deliberately no separate check for "same vertex subclass" (e.g. -rejecting a ``UVertex``-to-``DVertex`` connection directly). It doesn't need -one: since a ``UVertex`` can only ever belong to a ``UGraph`` and a -``DVertex`` only to a ``DGraph`` (enforced by ``add_vertex()`` above), the -same-graph check above already makes a ``UVertex``-to-``DVertex`` connection -impossible as a structural consequence, not as a separately-maintained rule. +There is no such thing as connecting an "undirected" vertex to a "directed" +one, since a vertex is neither: the kind of edge is the graph's, so the +same-graph check above is the only one needed. Constructing an ``Edge`` directly ----------------------------------- @@ -79,15 +76,17 @@ Summary * - Operation - Requires graph membership? - Failure mode - * - ``UVertex()`` / ``DVertex()`` / ``Vertex()`` + * - ``Vertex()`` - No - -- * - ``Edge(v1, v2)`` - No (cost falls back to ``None`` if ungraphed/cross-graph) - -- * - ``graph.add_vertex(v)`` - - ``v`` must be the graph's own vertex subclass, or coordinate data - - ``TypeError`` on wrong vertex subclass + - ``v`` must be of the graph's ``vertex_class`` and not already in a + graph, or coordinate data + - ``TypeError`` on a vertex of another class, ``ValueError`` if it is + already in a graph * - ``v1.connect(v2)`` / ``graph.add_edge(v1, v2)`` - Both vertices must already belong to the same graph - ``ValueError`` if either is ungraphed or they belong to different From 0182c197489f5eb92c85f1ce88c0a66effc6f257 Mon Sep 17 00:00:00 2001 From: Peter Corke Date: Mon, 12 Oct 2026 10:10:53 +1000 Subject: [PATCH 3/3] fix: Graph is concrete through its dispatch, not an abstract class Graph(directed=...) returns a UGraph or DGraph, so it is no longer abstract: the abstractmethod made static checkers report that an abstract class was instantiated. _graphcolor in the base raises NotImplementedError like the other hooks, and a direct subclass that does not say its kind is a TypeError at construction. Also tests the module-level deprecated names and the base hooks, and removes an unused import. Co-Authored-By: Claude Sonnet 5.5 --- src/pgraph/PGraph.py | 14 ++++++++-- tests/test_detach_hook.py | 2 +- tests/test_unified_graph.py | 56 +++++++++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 4 deletions(-) diff --git a/src/pgraph/PGraph.py b/src/pgraph/PGraph.py index f6e57011..7351806b 100644 --- a/src/pgraph/PGraph.py +++ b/src/pgraph/PGraph.py @@ -1,6 +1,5 @@ from __future__ import annotations -from abc import ABC, abstractmethod import os import sys import warnings @@ -46,7 +45,7 @@ def _SE2(v: NDArray) -> float: } -class Graph(ABC): +class Graph: #: whether the edges of this kind of graph are directed, set by #: :class:`UGraph` and :class:`DGraph` @@ -57,6 +56,11 @@ def __new__(cls, *args: Any, directed: bool | None = None, **kwargs: Any) -> Gra # underneath stay honest, so isinstance and the hooks work as usual if cls is Graph: cls = DGraph if directed else UGraph + elif not hasattr(cls, "_directed"): + raise TypeError( + f"{cls.__name__} does not say whether its edges are directed; " + "subclass UGraph or DGraph" + ) return super().__new__(cls) def __init__( @@ -1291,7 +1295,6 @@ def ne(self) -> int: """ return len(self._edges) - @abstractmethod def _graphcolor(self) -> int | None: """ Color the graph (subclass method) @@ -1299,7 +1302,12 @@ def _graphcolor(self) -> int | None: Concrete graph coloring algorithm, provided by :meth:`UGraph._graphcolor` and :meth:`DGraph._graphcolor`. + :raises NotImplementedError: not provided by this graph kind """ + raise NotImplementedError( + f"{type(self).__name__} does not define how to color the graph; " + "subclass UGraph or DGraph" + ) @property def nc(self) -> int: diff --git a/tests/test_detach_hook.py b/tests/test_detach_hook.py index f7e6c0d5..5dac0cd1 100644 --- a/tests/test_detach_hook.py +++ b/tests/test_detach_hook.py @@ -7,7 +7,7 @@ import unittest -from pgraph import DGraph, Edge, Graph, UGraph, Vertex +from pgraph import DGraph, Edge, Graph, UGraph class TestHookIsDefinedPerKind(unittest.TestCase): diff --git a/tests/test_unified_graph.py b/tests/test_unified_graph.py index b85de3cf..3259b7d0 100644 --- a/tests/test_unified_graph.py +++ b/tests/test_unified_graph.py @@ -194,6 +194,62 @@ def test_copy_and_pickle_keep_the_kind(self): self.assertEqual(h.validate(), []) +class TestNotInstantiatedDirectly(unittest.TestCase): + """Graph is concrete only through the kind it dispatches to""" + + def test_a_subclass_must_say_its_kind(self): + class Vague(Graph): + pass + + with self.assertRaises(TypeError): + Vague() + + def test_a_subclass_of_a_kind_is_fine(self): + class Mine(DGraph): + pass + + self.assertTrue(Mine().directed) + self.assertTrue(Mine(directed=True).directed) + + def test_the_base_hooks_say_which_kinds_provide_them(self): + g = UGraph() + a, b = g.add_vertex(name="a"), g.add_vertex(name="b") + e = Edge(a, b) + for call in ( + lambda: Graph._attach(g, e), + lambda: Graph._detach(g, e), + lambda: Graph._incoming(g, a), + lambda: Graph._graphcolor(g), + ): + with self.assertRaises(NotImplementedError): + call() + + def test_the_base_endpoint_test_is_directional(self): + g = DGraph() + a, b = g.add_vertex(name="a"), g.add_vertex(name="b") + e = Edge(a, b) + self.assertTrue(Graph._endpoints_match(g, e, a, b)) + self.assertFalse(Graph._endpoints_match(g, e, b, a)) + + +class TestModuleLevelDeprecatedNames(unittest.TestCase): + """The names are also reachable as attributes of pgraph.PGraph""" + + def test_alias_in_the_module(self): + import pgraph.PGraph as module + + with self.assertWarns(DeprecationWarning): + self.assertIs(module.UVertex, Vertex) + with self.assertWarns(DeprecationWarning): + self.assertIs(module.DVertex, Vertex) + + def test_other_names_in_the_module_are_errors(self): + import pgraph.PGraph as module + + with self.assertRaises(AttributeError): + module.NoSuchThing + + class TestEdgeArrow(unittest.TestCase): def test_arrow_follows_the_graph(self): for flag, arrow in ((False, "--"), (True, "->")):