diff --git a/autoarray/plot/array.py b/autoarray/plot/array.py index 7ff4e8c8c..b1de3efe2 100644 --- a/autoarray/plot/array.py +++ b/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 diff --git a/autoarray/plot/inversion.py b/autoarray/plot/inversion.py index c59c902d0..984f9aceb 100644 --- a/autoarray/plot/inversion.py +++ b/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, diff --git a/autoarray/plot/utils.py b/autoarray/plot/utils.py index a96367362..2f725f21f 100644 --- a/autoarray/plot/utils.py +++ b/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 diff --git a/test_autoarray/plot/test_utils.py b/test_autoarray/plot/test_utils.py index f2396d3b2..1b36351b4 100644 --- a/test_autoarray/plot/test_utils.py +++ b/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)