diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c5b799db..a35cfeab 100644 --- a/.github/workflows/ci.yml +++ b/.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: diff --git a/pyproject.toml b/pyproject.toml index a5b133ad..32cb28f3 100644 --- a/pyproject.toml +++ b/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 diff --git a/src/pgraph/PGraph.py b/src/pgraph/PGraph.py index 7ee665c4..b7c4ca89 100644 --- a/src/pgraph/PGraph.py +++ b/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,14 +1074,21 @@ 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. @@ -1089,8 +1096,12 @@ def plot( 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 ) diff --git a/tests/test_plot_args.py b/tests/test_plot_args.py new file mode 100644 index 00000000..435f8416 --- /dev/null +++ b/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()