refactor: one shared colour-norm helper (fixes inversion ignoring log10_min_value)

autoarray/plot/array.py

@@ -17,6 +17,7 @@
     numpy_grid,
     numpy_lines,
     numpy_positions,
+    norm_from,
     _apply_contours,
     _conf_imshow_origin,
 )
@@ -161,31 +162,7 @@ def plot_array(
         fig = ax.get_figure()
 
     # --- colour normalisation --------------------------------------------------
-    if use_log10:
-        try:
-            from autonerves import conf as _conf
-
-            log10_min = _conf.instance["visualize"]["general"]["general"][
-                "log10_min_value"
-            ]
-        except Exception:
-            log10_min = 1.0e-4
-        clipped = np.clip(array, log10_min, None)
-        vmin_log = vmin if (vmin is not None and np.isfinite(vmin)) else log10_min
-        if vmax is not None and np.isfinite(vmax):
-            vmax_log = vmax
-        else:
-            with np.errstate(all="ignore"):
-                vmax_log = np.nanmax(clipped)
-        if not np.isfinite(vmax_log) or vmax_log <= vmin_log:
-            vmax_log = vmin_log * 10.0
-        from matplotlib.colors import LogNorm
-        norm = LogNorm(vmin=vmin_log, vmax=vmax_log)
-    elif vmin is not None or vmax is not None:
-        from matplotlib.colors import Normalize
-        norm = Normalize(vmin=vmin, vmax=vmax)
-    else:
-        norm = None
+    norm = norm_from(array=array, use_log10=use_log10, vmin=vmin, vmax=vmax)
 
     # Compute the axes-box aspect ratio from the data extent so that the
     # physical cell is correctly shaped and tight_layout has no whitespace

autoarray/plot/inversion.py

@@ -7,9 +7,8 @@
 from typing import List, Optional, Tuple
 
 import numpy as np
-from matplotlib.colors import LogNorm, Normalize
 
-from autoarray.plot.utils import subplots, apply_extent, apply_labels, conf_figsize, save_figure, _conf_imshow_origin
+from autoarray.plot.utils import subplots, apply_extent, apply_labels, conf_figsize, save_figure, norm_from, _conf_imshow_origin
 
 
 def plot_inversion_reconstruction(
@@ -89,22 +88,7 @@ def plot_inversion_reconstruction(
         fig = ax.get_figure()
 
     # --- colour normalisation --------------------------------------------------
-    if use_log10:
-        vmin_log = vmin if (vmin is not None and np.isfinite(vmin)) else 1e-4
-        if vmax is not None and np.isfinite(vmax):
-            vmax_log = vmax
-        elif pixel_values is not None:
-            with np.errstate(all="ignore"):
-                vmax_log = float(np.nanmax(np.asarray(pixel_values)))
-            if not np.isfinite(vmax_log) or vmax_log <= vmin_log:
-                vmax_log = vmin_log * 10.0
-        else:
-            vmax_log = vmin_log * 10.0
-        norm = LogNorm(vmin=vmin_log, vmax=vmax_log)
-    elif vmin is not None or vmax is not None:
-        norm = Normalize(vmin=vmin, vmax=vmax)
-    else:
-        norm = None
+    norm = norm_from(array=pixel_values, use_log10=use_log10, vmin=vmin, vmax=vmax)
 
     extent = mapper.extent_from(
         values=pixel_values,

autoarray/plot/utils.py

@@ -267,6 +267,82 @@ def symmetric_cmap_from(array, symmetric_value=None):
     return colors.Normalize(vmin=-abs_max, vmax=abs_max)
 
 
+def norm_from(array=None, use_log10=False, vmin=None, vmax=None):
+    """Build the matplotlib colour norm for *array* from flat arguments.
+
+    The single implementation behind every colour scale the plot functions
+    apply: ``plot_array``, ``plot_inversion_reconstruction`` and (by
+    delegation) ``autogalaxy.util.plot_utils.norm_from``, which is what the
+    ``Clicker`` / ``Scribbler`` GUIs draw with. It used to be written out once
+    per call site, and the copies had diverged; the decisions taken when they
+    were merged are recorded below so a future reader knows which behaviour was
+    chosen deliberately.
+
+    **1. The log floor is always the configured one.** ``visualize``'s
+    ``general.general.log10_min_value`` is read (falling back to ``1.0e-4``
+    when config is unavailable) and the values are clipped to it before
+    ``vmax`` is derived. ``plot_inversion_reconstruction`` previously hardcoded
+    ``1.0e-4`` and never clipped, so a user who changed the configured floor
+    had it honoured on array plots and silently ignored on inversion plots.
+    That was a behaviour bug, and honouring config here is the fix.
+
+    **2. What *array* means is the caller's choice.** It is "the values being
+    coloured" — the image for ``plot_array``, the reconstruction's
+    ``pixel_values`` for ``plot_inversion_reconstruction``. The two call sites
+    genuinely colour different data, so this is not a divergence to reconcile.
+    ``array=None`` (an inversion with no pixel values) falls through to the
+    same widened-``vmin`` fallback as an all-``NaN`` array.
+
+    **3. A degenerate range is widened however it arose.** ``vmax <= vmin``
+    makes a ``LogNorm`` unusable whether the pair was derived or passed in
+    explicitly, so the widening is applied unconditionally.
+    ``plot_inversion_reconstruction`` previously guarded only its derived
+    branch, leaving an explicitly-passed degenerate pair to reach matplotlib.
+
+    Parameters
+    ----------
+    array
+        The values being coloured. Only read when *use_log10* is ``True`` and
+        no finite *vmax* is given; ``None`` is accepted and takes the fallback.
+    use_log10
+        When ``True`` a ``LogNorm`` is returned, floored at the configured
+        ``log10_min_value``.
+    vmin, vmax
+        Explicit colour-scale limits. When both are ``None`` and *use_log10* is
+        ``False``, ``None`` is returned and matplotlib applies its own default.
+
+    Returns
+    -------
+    matplotlib.colors.Normalize or None
+    """
+    if use_log10:
+        log10_min = _conf_log10_min_value()
+
+        vmin_log = vmin if (vmin is not None and np.isfinite(vmin)) else log10_min
+
+        if vmax is not None and np.isfinite(vmax):
+            vmax_log = vmax
+        elif array is None:
+            vmax_log = np.nan
+        else:
+            with np.errstate(all="ignore"):
+                vmax_log = np.nanmax(np.clip(array, log10_min, None))
+
+        if not np.isfinite(vmax_log) or vmax_log <= vmin_log:
+            vmax_log = vmin_log * 10.0
+
+        from matplotlib.colors import LogNorm
+
+        return LogNorm(vmin=vmin_log, vmax=vmax_log)
+
+    if vmin is not None or vmax is not None:
+        from matplotlib.colors import Normalize
+
+        return Normalize(vmin=vmin, vmax=vmax)
+
+    return None
+
+
 def set_with_color_values(ax, cmap, color_values, norm=None):
     """Attach a colorbar to *ax* driven by *color_values* rather than a plotted artist.
 
@@ -856,6 +932,16 @@ def _conf_output_format() -> str:
         return "show"
 
 
+def _conf_log10_min_value() -> float:
+    """Return the log10 colour-scale floor from config (``log10_min_value``)."""
+    try:
+        from autonerves import conf
+        general = conf.instance["visualize"]["general"]["general"]
+        return float(general["log10_min_value"])
+    except Exception:
+        return 1.0e-4
+
+
 def _conf_ticks(key: str, default: float) -> float:
     try:
         from autonerves import conf

test_autoarray/plot/test_utils.py

@@ -1,6 +1,19 @@
+from pathlib import Path
+
+import numpy as np
+import pytest
+from matplotlib.colors import LogNorm, Normalize
+
 from autonerves import conf
 
-from autoarray.plot.utils import _arcsec_labels
+import autoarray.plot as aplt
+from autoarray.plot import utils as plot_utils
+from autoarray.plot.utils import _arcsec_labels, norm_from
+
+
+@pytest.fixture(name="plot_path")
+def make_plot_path_setup():
+    return Path(Path(__file__).resolve().parent) / "files" / "plots"
 
 
 def test_arcsec_labels_default_suffix_format():
@@ -76,3 +89,169 @@ def test_arcsec_labels_minus_in_math():
     finally:
         ticks["symbol_over_decimal"] = original_symbol
         ticks["minus_in_math"] = original_minus
+
+
+class TestNormFrom:
+    """The one colour-norm helper `plot_array`, `plot_inversion_reconstruction`
+    and `autogalaxy.util.plot_utils.norm_from` all build their norms with.
+
+    Each behaviour below was one of the three call sites' before it was merged;
+    `norm_from`'s docstring records which won and why.
+    """
+
+    def test__no_limits_and_no_log__returns_none(self):
+        assert norm_from(array=np.array([1.0, 2.0, 3.0])) is None
+
+    def test__explicit_limits__returns_linear_norm(self):
+        norm = norm_from(array=np.array([1.0, 2.0, 3.0]), vmin=0.5, vmax=2.5)
+
+        assert isinstance(norm, Normalize)
+        assert not isinstance(norm, LogNorm)
+        assert norm.vmin == pytest.approx(0.5)
+        assert norm.vmax == pytest.approx(2.5)
+
+    def test__use_log10__explicit_limits_are_used_as_given(self):
+        norm = norm_from(
+            array=np.array([1.0, 2.0, 3.0]), use_log10=True, vmin=1.0e-3, vmax=1.0
+        )
+
+        assert isinstance(norm, LogNorm)
+        assert norm.vmin == pytest.approx(1.0e-3)
+        assert norm.vmax == pytest.approx(1.0)
+
+    def test__use_log10__vmax_derived_from_the_values_being_coloured(self):
+        norm = norm_from(array=np.array([1.0, 2.0, 7.0]), use_log10=True, vmin=1.0e-3)
+
+        assert norm.vmax == pytest.approx(7.0)
+
+    def test__use_log10__vmin_defaults_to_the_configured_floor(self):
+        general = conf.instance["visualize"]["general"]["general"]
+        original = general["log10_min_value"]
+        try:
+            general["log10_min_value"] = 3.0e-3
+
+            norm = norm_from(array=np.array([1.0, 2.0, 7.0]), use_log10=True)
+
+            assert norm.vmin == pytest.approx(3.0e-3)
+        finally:
+            general["log10_min_value"] = original
+
+    def test__use_log10__values_are_clipped_to_the_configured_floor(self):
+        """The floor is a floor on what is rendered, so `vmax` is derived from
+        the clipped values — a whole array below the floor scales as the floor,
+        not as its own (unrenderable) maximum."""
+        general = conf.instance["visualize"]["general"]["general"]
+        original = general["log10_min_value"]
+        try:
+            general["log10_min_value"] = 1.0e-2
+
+            norm = norm_from(
+                array=np.array([1.0e-5, 1.0e-4]), use_log10=True, vmin=1.0e-8
+            )
+
+            assert norm.vmax == pytest.approx(1.0e-2)
+        finally:
+            general["log10_min_value"] = original
+
+    def test__use_log10__degenerate_range_is_widened_even_when_passed_explicitly(self):
+        """`LogNorm(vmin=10, vmax=1)` is unusable however the pair arose, so the
+        widening is not confined to the derived-`vmax` branch."""
+        norm = norm_from(
+            array=np.array([1.0, 2.0]), use_log10=True, vmin=10.0, vmax=1.0
+        )
+
+        assert norm.vmin == pytest.approx(10.0)
+        assert norm.vmax == pytest.approx(100.0)
+
+    @pytest.mark.filterwarnings("ignore:All-NaN slice encountered:RuntimeWarning")
+    def test__use_log10__all_nan_values_still_yield_a_finite_range(self):
+        norm = norm_from(array=np.array([np.nan, np.nan]), use_log10=True)
+
+        assert np.isfinite(norm.vmin)
+        assert np.isfinite(norm.vmax)
+        assert norm.vmax > norm.vmin
+
+    @pytest.mark.filterwarnings("ignore:All-NaN slice encountered:RuntimeWarning")
+    def test__use_log10__no_values_at_all_takes_the_same_fallback(self):
+        """`plot_inversion_reconstruction` may have no `pixel_values`; that is
+        `array=None` here, and it must not raise."""
+        norm = norm_from(array=None, use_log10=True)
+        nan_norm = norm_from(array=np.array([np.nan]), use_log10=True)
+
+        assert norm.vmin == pytest.approx(nan_norm.vmin)
+        assert norm.vmax == pytest.approx(nan_norm.vmax)
+
+
+class TestBothCallSitesHonourTheConfiguredFloor:
+    """The regression this helper exists for.
+
+    `plot_inversion_reconstruction` used to hardcode `1e-4` and never read
+    `log10_min_value`, so a changed floor was honoured on array plots and
+    silently ignored on inversion plots. Both call sites are exercised for real
+    here — a spy wrapping the shared helper records the norm each one actually
+    applied.
+    """
+
+    FLOOR = 3.0e-3
+
+    @staticmethod
+    def _spy_on(monkeypatch, module):
+        """Wrap `module.norm_from` so the norm it returns can be inspected."""
+        recorded = []
+
+        def _recording_norm_from(**kwargs):
+            norm = plot_utils.norm_from(**kwargs)
+            recorded.append(norm)
+            return norm
+
+        monkeypatch.setattr(module, "norm_from", _recording_norm_from)
+        return recorded
+
+    def test__plot_array(self, array_2d_7x7, plot_path, plot_patch, monkeypatch):
+        from autoarray.plot import array as array_module
+
+        recorded = self._spy_on(monkeypatch, array_module)
+
+        general = conf.instance["visualize"]["general"]["general"]
+        original = general["log10_min_value"]
+        try:
+            general["log10_min_value"] = self.FLOOR
+
+            aplt.plot_array(
+                array=array_2d_7x7,
+                use_log10=True,
+                output_path=plot_path,
+                output_filename="array_log10_floor",
+                output_format="png",
+            )
+        finally:
+            general["log10_min_value"] = original
+
+        assert len(recorded) == 1
+        assert recorded[0].vmin == pytest.approx(self.FLOOR)
+
+    def test__plot_inversion_reconstruction(
+        self, rectangular_mapper_7x7_3x3, plot_path, plot_patch, monkeypatch
+    ):
+        from autoarray.plot import inversion as inversion_module
+
+        recorded = self._spy_on(monkeypatch, inversion_module)
+
+        general = conf.instance["visualize"]["general"]["general"]
+        original = general["log10_min_value"]
+        try:
+            general["log10_min_value"] = self.FLOOR
+
+            aplt.plot_inversion_reconstruction(
+                pixel_values=np.ones(9),
+                mapper=rectangular_mapper_7x7_3x3,
+                use_log10=True,
+                output_path=plot_path,
+                output_filename="inversion_log10_floor",
+                output_format="png",
+            )
+        finally:
+            general["log10_min_value"] = original
+
+        assert len(recorded) == 1
+        assert recorded[0].vmin == pytest.approx(self.FLOOR)