fix: validate the bin count in FalsificationResult.plot, delegate the rest (#284)

The `bins < 1` guard let a float like 2.5 through to a matplotlib
TypeError, which is what #284 reports. The same guard also raised
`TypeError: '<' not supported` on a strategy name or an edge sequence,
so neither ever reached matplotlib even though it accepts both.

Check only what the bin count owns: reject a non-positive integer, and
reject types matplotlib cannot take at all. Strategy names and edge
sequences now pass through and matplotlib validates them, which it does
more precisely ("'nonsense' is not a valid estimator for `bins`").

Widen the annotation to match what the parameter actually accepts. Add
falsification edge-case tests for the bins cases and for a
three-variable ~~ block.

Author: anevolbap

pathmc/falsify.py

@@ -46,6 +46,7 @@
 from __future__ import annotations
 
 import math
+from collections.abc import Sequence
 from dataclasses import dataclass
 from itertools import permutations
 from typing import TYPE_CHECKING
@@ -287,7 +288,7 @@ def _repr_html_(self) -> str:
     def plot(
         self,
         ax: matplotlib.axes.Axes | None = None,
-        bins: int | None = None,
+        bins: int | str | Sequence[float] | np.ndarray | None = None,
     ) -> matplotlib.figure.Figure:
         """Plot histograms of permuted-baseline violation fractions.
 
@@ -300,8 +301,11 @@ def plot(
         ----------
         ax : matplotlib.axes.Axes | None
             Axes to plot on. Creates a new figure if ``None``.
-        bins : int | None
-            Number of histogram bins. Defaults to an automatic choice.
+        bins : int | str | Sequence[float] | numpy.ndarray | None
+            Passed through to ``matplotlib.axes.Axes.hist``: a positive
+            integer bin count, a binning strategy name such as
+            ``"auto"``, or a sequence of bin edges. Defaults to an
+            automatic choice.
 
         Returns
         -------
@@ -312,6 +316,10 @@ def plot(
         ------
         RuntimeError
             If the result cannot be evaluated (no LMC tests).
+        ValueError
+            If *bins* is neither one of the accepted types nor, for an
+            integer bin count, positive. Strategy names and bin edges are
+            validated by matplotlib, which reports them more precisely.
         """
         import matplotlib.pyplot as plt
 
@@ -321,8 +329,19 @@ def plot(
                 "The DAG implies no testable conditional independences."
             )
 
-        if bins is not None and bins < 1:
-            raise ValueError(f"bins must be a positive integer or None, got {bins}.")
+        # Only the bin count is validated. A strategy name or an edge
+        # sequence is left to matplotlib, which reports both more precisely.
+        if bins is not None:
+            if isinstance(bins, bool) or not isinstance(
+                bins, (int, np.integer, Sequence, np.ndarray)
+            ):
+                raise ValueError(
+                    f"bins must be a positive integer, a binning strategy "
+                    f"name, a sequence of bin edges, or None, got {bins!r} "
+                    f"of type {type(bins).__name__}."
+                )
+            if isinstance(bins, (int, np.integer)) and bins < 1:
+                raise ValueError(f"bins must be a positive integer, got {bins}.")
 
         if ax is None:
             fig, ax = plt.subplots(figsize=(8, 4))

tests/test_falsify.py

@@ -995,6 +995,17 @@ def test_residual_covariance_rejected_via_method(self, confounded_data):
         with pytest.raises(ValueError, match="residual covariance"):
             m.falsify(random_seed=0)
 
+    def test_three_variable_residual_block_rejected(self, confounded_data):
+        # A chain of `~~` terms (X ~~ Y, Y ~~ Z) forms one three-variable
+        # block. Covered only transitively elsewhere; pin it explicitly.
+        df = confounded_data.assign(Z=confounded_data["Y"] * 0.5)
+        with pytest.raises(ValueError, match="residual covariance"):
+            falsify_graph(
+                build_graph(parse_spec("W ~ X + Y + Z\nX ~~ Y\nY ~~ Z")),
+                _to_nw(df),
+                random_seed=0,
+            )
+
     def test_dangling_residual_endpoint_rejected(self):
         # Y appears only in a ~~ term (not a DAG node): must raise cleanly,
         # not KeyError.
@@ -1088,13 +1099,38 @@ def test_explicit_huge_still_rejected(self):
 
 
 class TestPlotValidation:
-    def test_negative_bins_rejected(self):
+    @pytest.fixture
+    def result(self):
         import matplotlib
 
         matplotlib.use("Agg")
-        r = TestResultDisplay()._make(p_lmc=0.5, p_tpa=0.0, n_in_mec=0)
-        with pytest.raises(ValueError, match="bins"):
-            r.plot(bins=0)
+        return TestResultDisplay()._make(p_lmc=0.5, p_tpa=0.0, n_in_mec=0)
+
+    @pytest.mark.parametrize("bins", [-1, 0])
+    def test_non_positive_bin_count_rejected(self, result, bins):
+        with pytest.raises(ValueError, match="bins must be a positive integer"):
+            result.plot(bins=bins)
+
+    @pytest.mark.parametrize("bins", [2.5, True, {1, 2}])
+    def test_invalid_bins_type_rejected(self, result, bins):
+        # 2.5 previously slipped past the `bins < 1` guard and failed inside
+        # matplotlib; True would have plotted a single bin silently.
+        with pytest.raises(ValueError, match="bins must be a positive integer"):
+            result.plot(bins=bins)
+
+    @pytest.mark.parametrize("bins", [None, 10, np.int64(10)])
+    def test_bin_count_accepted(self, result, bins):
+        assert result.plot(bins=bins) is not None
+
+    @pytest.mark.parametrize("bins", ["auto", "sturges", [0.0, 0.5, 1.0], (0.0, 1.0)])
+    def test_strategy_and_edges_delegated_to_matplotlib(self, result, bins):
+        # The old `bins < 1` guard raised TypeError on these before
+        # matplotlib ever saw them.
+        assert result.plot(bins=bins) is not None
+
+    def test_invalid_strategy_name_reported_by_matplotlib(self, result):
+        with pytest.raises(ValueError, match="not a valid estimator"):
+            result.plot(bins="nonsense")
 
 
 class TestMiscEdgeCases: