diff --git a/src/pgraph/PGraph.py b/src/pgraph/PGraph.py index fc0f008..8f9f6af 100644 --- a/src/pgraph/PGraph.py +++ b/src/pgraph/PGraph.py @@ -1482,14 +1482,14 @@ def remove_vertex(self, vertex: Vertex) -> None: """ if vertex._graph is not self: raise ValueError("vertex does not belong to this graph") - assert vertex.name is not None + name = vertex._require_name() # its own lists name every edge at it, so no need to search the graph for edge in vertex._incident_edges(): self.remove_edge(edge) self._vertexlist.remove(vertex) - del self._vertexdict[vertex.name] + del self._vertexdict[name] vertex._graph = None # it is not in any graph now vertex._label = None # and so not in a component self._connectivitychange = True @@ -2273,7 +2273,8 @@ def near(self, coord: ArrayLike, radius: float) -> list[tuple[Vertex, float]]: found = [] for vertex in candidates: - assert vertex.coord is not None # the candidates all have a position + # type narrowing only: the candidates all have a position + assert vertex.coord is not None # nosec B101 dist = float(self.metric(vertex.coord - coord)) if dist <= radius: found.append((vertex, dist)) @@ -2339,19 +2340,19 @@ def _edge_layout(self) -> tuple[dict[Edge, float], dict[Edge, int]]: groups: dict[frozenset[int], list[Edge]] = {} loops: dict[Vertex, list[Edge]] = {} for e in self._edges: - assert e.v1 is not None and e.v2 is not None - if e.v1 is e.v2: - loops.setdefault(e.v1, []).append(e) + v1, v2 = e._ends() + if v1 is v2: + loops.setdefault(v1, []).append(e) else: - groups.setdefault(frozenset((index[e.v1], index[e.v2])), []).append(e) + groups.setdefault(frozenset((index[v1], index[v2])), []).append(e) rads: dict[Edge, float] = {} for edges in groups.values(): n = len(edges) for k, e in enumerate(edges): - assert e.v1 is not None and e.v2 is not None + v1, v2 = e._ends() offset = (k - (n - 1) / 2) * _ARC_STEP # the same side in space for an edge drawn the other way round - rads[e] = offset if index[e.v1] < index[e.v2] else -offset + rads[e] = offset if index[v1] < index[v2] else -offset loopno = {e: k for edges in loops.values() for k, e in enumerate(edges)} return rads, loopno @@ -2380,8 +2381,7 @@ def _edge_artist( from matplotlib.patches import FancyArrowPatch from matplotlib.transforms import Affine2D, ScaledTranslation from ._plot import _Arrow3D, _Loop3D, _loop_path - v1, v2 = edge.v1, edge.v2 - assert v1 is not None and v2 is not None + v1, v2 = edge._ends() shrink = markersize / 2 + 1 if v1 is v2: radius = _LOOP_RADIUS + _LOOP_GROWTH * loopno @@ -2627,10 +2627,10 @@ def plot( # the edges, each once rads, loopno = self._edge_layout() for e in self._edges: - assert e.v1 is not None + start = e._ends()[0] kwargs = dict(edge_kwargs) if color_components: - kwargs = {**dict(color=color[e.v1.label, :]), **kwargs} + kwargs = {**dict(color=color[start.label, :]), **kwargs} artist = self._edge_artist( ax, e, @@ -2656,10 +2656,8 @@ def plot( ) dx, dy = abs(x1 - x0), abs(y1 - y0) for e in loopno: - assert e.v1 is not None - ax.update_datalim( - [(e.v1.x - dx, e.v1.y - dy), (e.v1.x + dx, e.v1.y + dy)] - ) + v = e._ends()[0] + ax.update_datalim([(v.x - dx, v.y - dy), (v.x + dx, v.y + dy)]) ax.autoscale_view() ax.grid(grid) @@ -3443,11 +3441,11 @@ def incidence(self) -> NDArray: vdict[vertex] = i for j, e in enumerate(edges): - 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 not self.directed: - I[vdict[e.v1], j] = 2 # self-loop, counted at both ends + v1, v2 = e._ends() + I[vdict[v1], j] = 1 + I[vdict[v2], j] = 1 + if v1 is v2 and not self.directed: + I[vdict[v1], j] = 2 # self-loop, counted at both ends return I @@ -3800,8 +3798,7 @@ def path_UCS( parent_names: dict[str, str] = {} for v, p in parent.items(): - assert v.name is not None and p.name is not None - parent_names[v.name] = p.name + parent_names[v._require_name()] = p._require_name() if summary or verbose: print( @@ -3938,8 +3935,7 @@ def path_Astar( parent_names: dict[str, str] = {} for v, p in parent.items(): - assert v.name is not None and p.name is not None - parent_names[v.name] = p.name + parent_names[v._require_name()] = p._require_name() if summary or verbose: print( @@ -3980,9 +3976,9 @@ def _attach(self, edge: Edge) -> None: 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) + v1, v2 = edge._ends() + v1._edgelist.append(edge) + v2._edgelist.append(edge) def _detach(self, edge: Edge) -> None: """ @@ -4035,9 +4031,9 @@ def _attach(self, edge: Edge) -> None: 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) + v1, v2 = edge._ends() + v1._edgelist.append(edge) + v2._inlist.append(edge) def _detach(self, edge: Edge) -> None: """ @@ -4210,6 +4206,27 @@ def _set_endpoints(self, v1: Vertex | None, v2: Vertex | None) -> None: self._v1 = v1 self._v2 = v2 + def _ends(self) -> tuple[Vertex, Vertex]: + """ + Both vertices of the edge, raising clearly if it is not connected (private method) + + :raises ValueError: the edge has no vertices + :return: the first and second vertex + :rtype: tuple of Vertex subclass + + ``v1`` and ``v2`` are None for an edge that has not been connected. + Every edge in a graph has both, so the code that works on them takes + them from here, which tells the type checker so, rather than testing + for None at each use. Unlike an ``assert`` this check is not removed + by ``python -O``. + + :seealso: :attr:`v1` :attr:`v2` + """ + v1, v2 = self._v1, self._v2 + if v1 is None or v2 is None: + raise ValueError("the edge is not connected to two vertices") + return v1, v2 + def __reduce__(self) -> Any: """ Refuse to pickle or copy an edge on its own @@ -4341,12 +4358,11 @@ def next(self, vertex: Vertex) -> Vertex: """ - if self.v1 is vertex: - assert self.v2 is not None - return self.v2 - elif self.v2 is vertex: - assert self.v1 is not None - return self.v1 + v1, v2 = self._ends() + if v1 is vertex: + return v2 + elif v2 is vertex: + return v1 else: raise ValueError("should not happen") @@ -4385,8 +4401,7 @@ def endpoints(self) -> list[Vertex]: :seealso: :meth:`vertices` """ - assert self.v1 is not None and self.v2 is not None - return [self.v1, self.v2] + return list(self._ends()) def remove(self) -> None: """ @@ -4811,7 +4826,8 @@ def _bind_edge(self, edge: Edge, dest: Vertex) -> None: endpoints disagree with the vertices' edge lists. """ graph = self._graph - assert graph is not None # connect() has checked + # type narrowing only: connect() has checked + assert graph is not None # nosec B101 if edge.v1 is None and edge.v2 is None: edge._set_endpoints(self, dest) elif not graph._endpoints_match(edge, self, dest): @@ -5148,6 +5164,25 @@ def _incident_edges(self) -> list[Edge]: seen.setdefault(id(e), e) return list(seen.values()) + def _require_name(self) -> str: + """ + The name, raising clearly if the vertex has none (private method) + + :raises ValueError: the vertex has no name + :return: the name + :rtype: str + + A vertex that is not in a graph can have no name, but one in a graph + always has, so the code that works on the vertices of a graph takes + the name from here, which tells the type checker so. Unlike an + ``assert`` this check is not removed by ``python -O``. + + :seealso: :attr:`name` + """ + if self._name is None: + raise ValueError("the vertex has no name") + return self._name + @property def name(self) -> str | None: """ diff --git a/tests/test_edge_endpoints.py b/tests/test_edge_endpoints.py index a31add9..64acfbb 100644 --- a/tests/test_edge_endpoints.py +++ b/tests/test_edge_endpoints.py @@ -120,5 +120,42 @@ def test_repr_shows_the_endpoints_and_an_edge_alone_is_not_copied(self): self.assertEqual((dup.v1.name, dup.v2.name), ("a", "b")) +class TestUnconnected(unittest.TestCase): + """The code that works on both ends of an edge says so if there are none""" + + def test_a_connected_edge_gives_its_ends(self): + g = UGraph() + a = g.add_vertex([0, 0], name="a") + b = g.add_vertex([1, 0], name="b") + e = a.connect(b) + self.assertEqual(e.endpoints, [a, b]) + self.assertIs(e.next(a), b) + self.assertIs(e.next(b), a) + + def test_an_edge_with_no_vertices_raises_clearly(self): + e = Edge() + self.assertIsNone(e.v1) + self.assertIsNone(e.v2) + v = UGraph().add_vertex([0, 0], name="a") + for name, call in ( + ("_ends", e._ends), + ("endpoints", lambda: e.endpoints), + ("next", lambda: e.next(v)), + ): + with self.subTest(name=name): + with self.assertRaises(ValueError) as cm: + call() + self.assertIn("not connected", str(cm.exception)) + + def test_next_of_a_vertex_that_is_not_an_end(self): + g = UGraph() + a = g.add_vertex([0, 0], name="a") + b = g.add_vertex([1, 0], name="b") + c = g.add_vertex([2, 0], name="c") + e = a.connect(b) + with self.assertRaises(ValueError): + e.next(c) + + if __name__ == "__main__": unittest.main() diff --git a/tests/test_vertex_init.py b/tests/test_vertex_init.py index 9a4a3cf..2fc76b1 100644 --- a/tests/test_vertex_init.py +++ b/tests/test_vertex_init.py @@ -161,5 +161,19 @@ def test_assigning_other_attributes_is_not_a_change(self): self.assertFalse(g._spatialchange) +class TestRequireName(unittest.TestCase): + """The code that works on the vertices of a graph takes their names from here""" + + def test_a_named_vertex_gives_its_name(self): + self.assertEqual(Vertex(name="a")._require_name(), "a") + # a vertex in a graph always has one: the default is "#0", "#1", ... + self.assertEqual(UGraph().add_vertex()._require_name(), "#0") + + def test_a_vertex_with_no_name_raises_clearly(self): + with self.assertRaises(ValueError) as cm: + Vertex()._require_name() + self.assertIn("no name", str(cm.exception)) + + if __name__ == "__main__": unittest.main()