fix: plot()/highlight_edge() crashes found by mypy; mypy to zero

.github/workflows/ci.yml

@@ -40,12 +40,6 @@ jobs:
           python -m pip install --upgrade pip
           pip install .[dev]
       - name: Run mypy
-        # Informational only for now: the type-hinting pass surfaced a
-        # baseline of pre-existing design tensions (Liskov-incompatible
-        # overrides, embedded-graph invariants not visible to the type
-        # checker) that are tracked separately, not fixed here. Don't fail
-        # the build on them yet.
-        continue-on-error: true
         run: mypy src/pgraph/PGraph.py
 
   all-tests-passed:

pyproject.toml

@@ -76,7 +76,6 @@ target-version = ['py311']
 omit = []
 
 [tool.mypy]
-python_version = "3.9"
 ignore_missing_imports = true
 warn_unused_ignores = true
 warn_redundant_casts = true

src/pgraph/PGraph.py

@@ -1062,10 +1062,10 @@ def plot(
         self,
         colorcomponents: bool = True,
         force2d: bool = False,
-        vopt: dict = {},
-        eopt: dict = {},
-        text: dict | bool = {},
-        block: bool = False,
+        vopt: dict | None = None,
+        eopt: dict | None = None,
+        text: dict | bool | None = None,
+        block: bool | None = False,
         grid: bool = True,
         ax: Any = None,
     ) -> None:
@@ -1074,23 +1074,34 @@ def plot(
 
         :param vopt: vertex format, defaults to 12pt o-marker
         :type vopt: dict, optional
-        :param eopt: edge format, defaults to None
+        :param eopt: edge format, defaults to 3pt line width
         :type eopt: dict, optional
         :param text: text label format, defaults to None
-        :type text: False or dict, optional
-        :param colorcomponents: color vertices and edges by component, defaults to None
-        :type color: bool, optional
-        :param block: block until figure is dismissed, defaults to True
-        :type block: bool, optional
+        :type text: dict, bool or None, optional
+        :param colorcomponents: color vertices and edges by component, defaults to True
+        :type colorcomponents: bool, optional
+        :param force2d: plot only the first two coordinates of each vertex
+            even if they have more, defaults to False
+        :type force2d: bool, optional
+        :param block: block until figure is dismissed, defaults to False. If
+            None the figure is not shown at all, which is useful when
+            adding to the plot or saving it.
+        :type block: bool or None, optional
+        :raises ValueError: the graph is empty, or a vertex has no coordinate
+            or fewer than two coordinate values
 
         The graph is plotted using matplotlib.
 
         If ``colorcomponents`` is True then each component is assigned a unique
         color.  ``vertex`` and ``edge`` cannot include a color keyword.
 
         If ``text`` is a dict it is used to format text labels for the vertices
-        which are the vertex names.  If ``text`` is None default formatting is
-        used.  If ``text`` is False no labels are added.
+        which are the vertex names.  If ``text`` is None or True default
+        formatting is used.  If ``text`` is False no labels are added.
+
+        .. note:: Only embedded graphs can be plotted: every vertex must have
+            a coordinate of at least two values.  A coordinate of more than
+            two values is drawn in 3D, using the first three.
 
         .. runblock:: pycon
 
@@ -1112,21 +1123,37 @@ def plot(
 
         :seealso: :meth:`highlight_path`
         """
-        vopt = {**dict(marker="o", markersize=12), **vopt}
-        eopt = {**dict(linewidth=3), **eopt}
+        if self.n == 0:
+            raise ValueError("cannot plot an empty graph")
+        ndim = 0  # number of coordinate values, taken from the first vertex
+        for i, vertex in enumerate(self):
+            if vertex.coord is None or len(vertex.coord) < 2:
+                raise ValueError(
+                    f"cannot plot: vertex {vertex.name} needs a coordinate "
+                    "of at least two values"
+                )
+            if i == 0:
+                ndim = len(vertex.coord)
+
+        vopt = {**dict(marker="o", markersize=12), **(vopt or {})}
+        eopt = {**dict(linewidth=3), **(eopt or {})}
+        # text labels: None or True is default formatting, False is no labels
+        textopt: dict | None = None
+        if text is not False:
+            textopt = text if isinstance(text, dict) else {}
 
         if colorcomponents:
             color = plt.cm.coolwarm(np.linspace(0, 1, self.nc))
 
-        if len(self[0].coord) == 2 or force2d:
+        if ndim == 2 or force2d:
             # 2D plotting
             if ax is None:
                 ax = axes_logic(ax, 2)
             for c in range(self.nc):
                 # for each component
                 for vertex in self.component(c):
-                    if text is not False:
-                        ax.text(vertex.x, vertex.y, "  " + vertex.name, **text)
+                    if textopt is not None:
+                        ax.text(vertex.x, vertex.y, f"  {vertex.name}", **textopt)
                     if colorcomponents:
                         ax.plot(vertex.x, vertex.y, color=color[c, :], **vopt)
                         for v in vertex.neighbours():
@@ -1147,9 +1174,9 @@ def plot(
             for c in range(self.nc):
                 # for each component
                 for vertex in self.component(c):
-                    if text is not False:
+                    if textopt is not None:
                         ax.text(
-                            vertex.x, vertex.y, vertex.z, "  " + vertex.name, **text
+                            vertex.x, vertex.y, vertex.z, f"  {vertex.name}", **textopt
                         )
                     if colorcomponents:
                         ax.plot(
@@ -1166,7 +1193,7 @@ def plot(
                                 **{**dict(color=color[c, :]), **eopt},
                             )
                     else:
-                        ax.plot(vertex.x, vertex.y, **vopt)
+                        ax.plot(vertex.x, vertex.y, vertex.z, **vopt)
                         for v in vertex.neighbours():
                             ax.plot(
                                 [vertex.x, v.x],
@@ -1181,13 +1208,17 @@ def plot(
         if block is not None:
             plt.show(block=block)
 
-    def highlight_path(self, path: list[BaseVertex], block: bool = False, **kwargs: Any) -> None:
+    def highlight_path(
+        self, path: list[BaseVertex], block: bool | None = False, **kwargs: Any
+    ) -> None:
         """
         Highlight a path through the graph
 
         :param path: sequence of vertices forming a path
         :type path: list of BaseVertex subclass
-        :param block: block until figure is dismissed, defaults to False
+        :param block: block until figure is dismissed, defaults to False. If
+            None the figure is not shown.
+        :type block: bool or None, optional
         :param kwargs: arguments passed to :meth:`highlight_edge` and
             :meth:`highlight_vertex`
 
@@ -1243,6 +1274,8 @@ def highlight_edge(
         :type color: str, optional
         :param alpha: Transparency of the highlight, defaults to 0.5
         :type alpha: float, optional
+        :raises ValueError: ``edge`` does not connect two vertices, for
+            example because it has been removed from the graph
         :rtype: None
 
         .. runblock:: pycon
@@ -1271,6 +1304,8 @@ def highlight_edge(
         """
         p1 = edge.v1
         p2 = edge.v2
+        if p1 is None or p2 is None:
+            raise ValueError("edge does not connect two vertices")
         plt.plot(
             [p1.x, p2.x], [p1.y, p2.y], color=color, linewidth=3 * scale, alpha=alpha
         )

tests/test_plot_args.py

@@ -0,0 +1,112 @@
+"""
+plot() / highlight_*() argument handling and the errors they give for graphs
+that cannot be drawn.  These were found by mypy: ``text=None`` and
+``text=True`` crashed although the docstring documents ``None`` as "default
+formatting", and unplottable graphs gave a bare TypeError/IndexError.
+"""
+
+import unittest
+
+import matplotlib
+
+matplotlib.use("Agg")
+import matplotlib.pyplot as plt
+
+from pgraph import DGraph, UGraph
+
+
+class TestPlotArgs(unittest.TestCase):
+    def setUp(self):
+        self.g = UGraph()
+        a = self.g.add_vertex([0, 0], name="a")
+        b = self.g.add_vertex([1, 1], name="b")
+        self.edge = self.g.add_edge(a, b)
+
+    def tearDown(self):
+        plt.close("all")
+
+    def texts(self):
+        return [t.get_text().strip() for t in plt.gca().texts]
+
+    def test_text_default_labels_vertices(self):
+        self.g.plot(block=None)
+        self.assertEqual(sorted(self.texts()), ["a", "b"])
+
+    def test_text_none_means_default_formatting(self):
+        self.g.plot(text=None, block=None)
+        self.assertEqual(sorted(self.texts()), ["a", "b"])
+
+    def test_text_true_means_default_formatting(self):
+        self.g.plot(text=True, block=None)
+        self.assertEqual(sorted(self.texts()), ["a", "b"])
+
+    def test_text_false_means_no_labels(self):
+        self.g.plot(text=False, block=None)
+        self.assertEqual(self.texts(), [])
+
+    def test_text_dict_is_the_format(self):
+        self.g.plot(text={"color": "r"}, block=None)
+        self.assertEqual({t.get_color() for t in plt.gca().texts}, {"r"})
+
+    def test_vopt_eopt_none(self):
+        self.g.plot(vopt=None, eopt=None, block=None)
+
+    def test_options_are_not_shared_between_calls(self):
+        # a mutable default argument must not accumulate state
+        self.g.plot(vopt={"markersize": 3}, block=None)
+        plt.close("all")
+        self.g.plot(block=None)
+        sizes = {line.get_markersize() for line in plt.gca().lines}
+        self.assertNotIn(3, sizes)
+
+    def test_3d_text_none(self):
+        g = UGraph()
+        a = g.add_vertex([0, 0, 0], name="a")
+        b = g.add_vertex([1, 1, 1], name="b")
+        g.add_edge(a, b)
+        g.plot(text=None, block=None)
+        self.assertEqual(sorted(self.texts()), ["a", "b"])
+
+    def test_3d_vertices_plotted_at_their_z(self):
+        # without component colouring the vertex marker dropped z (plotted at 0)
+        g = UGraph()
+        a = g.add_vertex([0, 0, 5], name="a")
+        b = g.add_vertex([1, 1, 7], name="b")
+        g.add_edge(a, b)
+        g.plot(colorcomponents=False, block=None)
+        zs = set()
+        for line in plt.gca().lines:
+            x, y, z = line.get_data_3d()
+            if len(x) == 1:  # a vertex marker, not an edge
+                zs.add(float(z[0]))
+        self.assertEqual(zs, {5.0, 7.0})
+
+    def test_plot_graph_without_coordinates(self):
+        g = UGraph()
+        g.add_edge(g.add_vertex(), g.add_vertex())
+        with self.assertRaises(ValueError):
+            g.plot(block=None)
+
+    def test_plot_graph_with_one_coordinate(self):
+        g = UGraph()
+        g.add_vertex([0])
+        with self.assertRaises(ValueError):
+            g.plot(block=None)
+
+    def test_plot_empty_graph(self):
+        with self.assertRaises(ValueError):
+            DGraph().plot(block=None)
+
+    def test_highlight_edge(self):
+        self.g.plot(block=None)
+        self.g.highlight_edge(self.edge)
+
+    def test_highlight_removed_edge(self):
+        self.g.plot(block=None)
+        self.g.remove_edge(self.edge)
+        with self.assertRaises(ValueError):
+            self.g.highlight_edge(self.edge)
+
+
+if __name__ == "__main__":
+    unittest.main()