From 3fee40980fc3ea69bd901180fc435665cbdcbc3b Mon Sep 17 00:00:00 2001 From: Jonathan Nightingale Date: Mon, 24 Aug 2026 21:59:53 +0000 Subject: [PATCH 1/2] fix: make the GUI helpers' colour scale constructible again `Cmap` is no longer exported by any public plot namespace (autoarray.plot, autogalaxy.plot, autolens.plot, autocti.plot). Both GUI helpers still required one, so their colour scale could only be supplied via a private-path import: - `Clicker.start()` built `aplt.Cmap(...)` on `autoarray.plot` itself (clicker.py:31), so it raised `AttributeError: module 'autoarray.plot' has no attribute 'Cmap'` for every caller. - `Scribbler.__init__` took a `Cmap`-shaped object as its `cmap=` argument, leaving callers no public way to colour the GUI. Replace the object with flat arguments matching the `plot_array` convention: - Add `norm_from(array, use_log10, vmin, vmax)` to `util/plot_utils.py`, mirroring the normalisation `autoarray.plot.array.plot_array` applies, so code drawing its own axes scales colour the same way the plot functions do. - `Scribbler.__init__` gains `norm` / `vmin` / `vmax` and accepts a colormap name for `cmap`. A legacy `Cmap`-style object still works, so any caller holding an instance is not broken. - `Clicker.start()` uses the helper directly; its hardcoded scale is unchanged. - Drop the duplicate `Scribbler` import in `__init__.py` (it appeared at both line 76 and line 124), keeping the one grouped with `Clicker`. Adds `test_autogalaxy/gui/`, which did not exist: nine tests over the norm construction, the new signature, and a regression guard asserting `Cmap` stays absent from the public namespaces. `Scribbler.__init__` itself is not covered because it needs TkAgg and a display; the extracted helper carries the logic. Full suite green on Python 3.12 (1122 passed); 3.13 is left to CI. Note for a follow-up, not fixed here: autoarray duplicates this same normalisation inline in `plot/array.py` and `plot/inversion.py`. A shared helper there would be the real fix; this adds one copy in autogalaxy serving both GUIs rather than a third inline copy. Refs PyAutoGalaxy#585 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LN2Qsx6JjVKV45o17EKGtB --- autogalaxy/__init__.py | 1 - autogalaxy/gui/clicker.py | 7 ++- autogalaxy/gui/scribbler.py | 42 ++++++++++++-- autogalaxy/util/plot_utils.py | 60 ++++++++++++++++++++ test_autogalaxy/gui/__init__.py | 0 test_autogalaxy/gui/test_plot_norm.py | 82 +++++++++++++++++++++++++++ 6 files changed, 184 insertions(+), 8 deletions(-) create mode 100644 test_autogalaxy/gui/__init__.py create mode 100644 test_autogalaxy/gui/test_plot_norm.py diff --git a/autogalaxy/__init__.py b/autogalaxy/__init__.py index 1584430d0..8cafa3c7a 100644 --- a/autogalaxy/__init__.py +++ b/autogalaxy/__init__.py @@ -73,7 +73,6 @@ from .operate.image import OperateImageList from .operate.image import OperateImageGalaxies from .operate.lens_calc import LensCalc -from .gui.scribbler import Scribbler from .imaging.fit_imaging import FitImaging from .imaging.model.analysis import AnalysisImaging from autofit import Latent diff --git a/autogalaxy/gui/clicker.py b/autogalaxy/gui/clicker.py index a3f634ecd..20bcfbe48 100644 --- a/autogalaxy/gui/clicker.py +++ b/autogalaxy/gui/clicker.py @@ -21,15 +21,16 @@ def __init__(self, image, pixel_scales, search_box_size, in_pixels: bool = False def start(self, data, pixel_scales): from matplotlib import pyplot as plt - import autoarray.plot as aplt from autoarray.plot.utils import _conf_imshow_origin + from autogalaxy.util.plot_utils import norm_from n_y, n_x = data.shape_native hw = int(n_x / 2) * pixel_scales ext = [-hw, hw, -hw, hw] fig = plt.figure(figsize=(14, 14)) - cmap = aplt.Cmap(cmap="jet", norm="log", vmin=1.0e-3, vmax=np.max(data) / 3.0) - norm = cmap.norm_from(array=data, use_log10=True) + norm = norm_from( + array=data, use_log10=True, vmin=1.0e-3, vmax=np.max(data) / 3.0 + ) plt.imshow(data.native, cmap="jet", norm=norm, extent=ext, origin=_conf_imshow_origin()) if not data.mask.is_all_false: grid = data.mask.derive_grid.edge diff --git a/autogalaxy/gui/scribbler.py b/autogalaxy/gui/scribbler.py index 9437f6d98..ac0f04b84 100644 --- a/autogalaxy/gui/scribbler.py +++ b/autogalaxy/gui/scribbler.py @@ -9,6 +9,9 @@ def __init__( image, segment_names=None, cmap=None, + norm=None, + vmin=None, + vmax=None, brush_width=0.05, backend="TkAgg", mask_overlay=None, @@ -22,6 +25,19 @@ def __init__( the gui folder for a description. This script is Adapted from https://gist.github.com/brikeats/4f63f867fd8ea0f196c78e9b835150ab + + Parameters + ---------- + cmap + The colormap name, e.g. ``"jet"``. ``None`` or ``"default"`` uses + the configured default. A legacy ``Cmap``-style object exposing + ``norm_from`` is still accepted and takes precedence over the + *norm* / *vmin* / *vmax* arguments below. + norm + ``"log"`` for a logarithmic colour scale, ``"linear"`` (or ``None``) + otherwise. + vmin, vmax + Explicit colour-scale limits. """ if extent is not None: @@ -74,12 +90,30 @@ def __init__( plt.imshow(rgb_image, origin=_conf_imshow_origin()) self.ax = self.figure.add_subplot(111) - if cmap is None: + if cmap is None and norm is None and vmin is None and vmax is None: plt.imshow(image, interpolation="none", origin=_conf_imshow_origin()) + elif hasattr(cmap, "norm_from"): + # Legacy `Cmap`-style object. The public plot namespaces no longer + # export one, but a caller holding an instance still works. + mpl_norm = cmap.norm_from(array=image) + cmap_name = getattr(cmap, "cmap_name", None) or cmap.config_dict.get( + "cmap", "viridis" + ) + plt.imshow( + image, cmap=cmap_name, norm=mpl_norm, origin=_conf_imshow_origin() + ) else: - norm = cmap.norm_from(array=image) - cmap_name = getattr(cmap, "cmap_name", None) or cmap.config_dict.get("cmap", "viridis") - plt.imshow(image, cmap=cmap_name, norm=norm, origin=_conf_imshow_origin()) + from autogalaxy.util.plot_utils import _resolve_colormap, norm_from + + mpl_norm = norm_from( + array=image, use_log10=norm == "log", vmin=vmin, vmax=vmax + ) + plt.imshow( + image, + cmap=_resolve_colormap(cmap), + norm=mpl_norm, + origin=_conf_imshow_origin(), + ) if mask_overlay is not None: grid = mask_overlay.derive_grid.edge diff --git a/autogalaxy/util/plot_utils.py b/autogalaxy/util/plot_utils.py index 528f08392..24d761ea2 100644 --- a/autogalaxy/util/plot_utils.py +++ b/autogalaxy/util/plot_utils.py @@ -102,6 +102,66 @@ def _resolve_colormap(colormap): return colormap +def norm_from(array, use_log10=False, vmin=None, vmax=None): + """Build the matplotlib colour norm for *array* from flat arguments. + + This is the flat-API replacement for the removed ``Cmap`` object's + ``norm_from`` method. It exists so that code drawing its own axes — the + ``Clicker`` and ``Scribbler`` GUIs — scales colour the same way the + ``plot_*`` functions do, without needing a plotter object that the public + namespaces no longer export. + + The behaviour mirrors the normalisation applied inside + ``autoarray.plot.array.plot_array``. + + Parameters + ---------- + array + The image being normalised. Only read when *use_log10* is ``True`` and + no explicit *vmax* is given. + use_log10 + When ``True`` a ``LogNorm`` is applied, with values clipped at the + configured ``log10_min_value`` floor. + 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: + 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 + + 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 _resolve_format(output_format): """Normalise output_format: accept a list/tuple or a plain string.""" from autoarray.plot.utils import _conf_output_format diff --git a/test_autogalaxy/gui/__init__.py b/test_autogalaxy/gui/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/test_autogalaxy/gui/test_plot_norm.py b/test_autogalaxy/gui/test_plot_norm.py new file mode 100644 index 000000000..b6a68efad --- /dev/null +++ b/test_autogalaxy/gui/test_plot_norm.py @@ -0,0 +1,82 @@ +import numpy as np +import pytest +from matplotlib.colors import LogNorm, Normalize + +import autogalaxy as ag +from autogalaxy.util.plot_utils import norm_from + + +class TestNormFrom: + def test__no_arguments__returns_none(self): + assert norm_from(array=np.array([1.0, 2.0, 3.0])) is None + + def test__vmin_vmax__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__returns_log_norm_with_explicit_limits(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_array(self): + norm = norm_from(array=np.array([1.0, 2.0, 7.0]), use_log10=True, vmin=1.0e-3) + + assert isinstance(norm, LogNorm) + assert norm.vmax == pytest.approx(7.0) + + def test__use_log10__degenerate_range_is_widened(self): + """vmax <= vmin would make LogNorm unusable, so it is pushed up a decade.""" + 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_array_still_yields_finite_limits(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 + + +class TestPublicNamespace: + def test__gui_classes_are_exported_once(self): + assert hasattr(ag, "Scribbler") + assert hasattr(ag, "Clicker") + + def test__scribbler_accepts_flat_colour_arguments(self): + """The removed `Cmap` object was the only way to colour these GUIs. + + `Scribbler` must therefore take the colour scale as plain values, since + no public plot namespace exports a `Cmap` to hand it any more. + """ + import inspect + + params = inspect.signature(ag.Scribbler.__init__).parameters + + for name in ("cmap", "norm", "vmin", "vmax"): + assert name in params, f"Scribbler.__init__ is missing `{name}`" + + def test__cmap_is_absent_from_every_public_plot_namespace(self): + """Regression guard for the defect this module was added for. + + If `Cmap` ever returns to a public namespace, the GUIs' flat arguments + are no longer the only option and this test should be revisited. + """ + import autoarray.plot as aaplt + import autogalaxy.plot as agplt + + assert not hasattr(aaplt, "Cmap") + assert not hasattr(agplt, "Cmap") From 3749aed277054a53f6f65caad4938b98ddcd5375 Mon Sep 17 00:00:00 2001 From: Jonathan Nightingale Date: Mon, 24 Aug 2026 22:12:30 +0000 Subject: [PATCH 2/2] fix: let plot_array overlay a mask that is not the array's own MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `plot_array` auto-derives its mask outline from `array.mask`, which yields nothing for an unmasked array. The autogalaxy wrapper did not forward autoarray's `mask=` parameter, so a caller wanting to outline a *different* mask — the common preprocessing case of showing a candidate mask radius over unmasked data — had no way to ask for it through the public API. Add the `mask=` passthrough plus `_mask_edge`, which accepts a `Mask2D` (or coordinates already in edge form) and mirrors `auto_mask_edge`'s contract of returning None when there is no edge to draw. Verified by rendering: on an unmasked array `auto_mask_edge` returns None and the outline is absent, while passing `mask=` changes 2436 pixels of the output PNG. Seven tests added in test_autogalaxy/plot/test_plot_array_mask.py. Full suite green on Python 3.12 (1129 passed). Refs PyAutoGalaxy#585 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LN2Qsx6JjVKV45o17EKGtB --- autogalaxy/util/plot_utils.py | 28 +++++++ test_autogalaxy/plot/test_plot_array_mask.py | 88 ++++++++++++++++++++ 2 files changed, 116 insertions(+) create mode 100644 test_autogalaxy/plot/test_plot_array_mask.py diff --git a/autogalaxy/util/plot_utils.py b/autogalaxy/util/plot_utils.py index 24d761ea2..c9902e93f 100644 --- a/autogalaxy/util/plot_utils.py +++ b/autogalaxy/util/plot_utils.py @@ -102,6 +102,28 @@ def _resolve_colormap(colormap): return colormap +def _mask_edge(mask): + """Convert a mask to the ``(N, 2)`` edge coordinates ``plot_array`` overlays. + + Accepts a ``Mask2D`` (or anything exposing ``derive_grid.edge``) as well as + coordinates that are already in that form. Mirrors the contract of + ``autoarray.plot.utils.auto_mask_edge``, returning ``None`` when there is no + edge to draw. + """ + if mask is None: + return None + try: + if mask.is_all_false: + return None + return np.array(mask.derive_grid.edge.array) + except AttributeError: + pass + try: + return np.asarray(mask) + except Exception: + return None + + def norm_from(array, use_log10=False, vmin=None, vmax=None): """Build the matplotlib colour norm for *array* from flat arguments. @@ -196,6 +218,7 @@ def plot_array( lines=None, line_colors=None, grid=None, + mask=None, cb_unit=None, ax=None, ): @@ -237,6 +260,10 @@ def plot_array( Colours for each entry in *lines*. grid : array-like or None An additional grid of points to overlay. + mask : Mask2D or array-like or None + A mask whose edge is overlaid as black dots. Pass this when the outline + wanted is *not* the array's own mask — for an already-masked array the + edge is derived automatically and this can be left ``None``. ax : matplotlib.axes.Axes or None Existing ``Axes`` to draw into. When provided the figure is *not* saved — the caller is responsible for saving. @@ -266,6 +293,7 @@ def plot_array( _aa_plot_array( array=array, ax=ax, + mask=_mask_edge(mask), grid=_numpy_grid(grid), positions=_positions_list, lines=_lines_list, diff --git a/test_autogalaxy/plot/test_plot_array_mask.py b/test_autogalaxy/plot/test_plot_array_mask.py new file mode 100644 index 000000000..bd94ab6d0 --- /dev/null +++ b/test_autogalaxy/plot/test_plot_array_mask.py @@ -0,0 +1,88 @@ +import numpy as np +import pytest + +import autogalaxy as ag +import autogalaxy.plot as aplt +from autogalaxy.util.plot_utils import _mask_edge + + +@pytest.fixture +def array(): + return ag.Array2D.no_mask( + values=np.abs(np.random.rand(30, 30)) + 1.0e-3, pixel_scales=0.1 + ) + + +@pytest.fixture +def mask(array): + return ag.Mask2D.circular( + shape_native=array.shape_native, pixel_scales=array.pixel_scales, radius=1.0 + ) + + +class TestMaskEdge: + def test__none_returns_none(self): + assert _mask_edge(None) is None + + def test__fully_unmasked_returns_none(self, array): + """Nothing to outline, matching `auto_mask_edge`'s contract.""" + assert _mask_edge(array.mask) is None + + def test__mask2d_returns_edge_coordinates(self, mask): + edge = _mask_edge(mask) + + assert isinstance(edge, np.ndarray) + assert edge.ndim == 2 and edge.shape[1] == 2 + assert len(edge) > 0 + + def test__raw_coordinates_pass_through(self): + coords = np.array([[1.0, 2.0], [3.0, 4.0]]) + + assert _mask_edge(coords) == pytest.approx(coords) + + +class TestPlotArrayMaskOverlay: + def test__unmasked_array_derives_no_overlay(self, array): + """The reason an explicit `mask=` is needed at all. + + `plot_array` auto-derives the outline from the array's own mask, which + yields nothing for an unmasked array — so a caller wanting a *different* + mask outlined must pass it. + """ + from autoarray.plot.utils import auto_mask_edge + + assert auto_mask_edge(array) is None + + def test__explicit_mask_changes_the_rendered_figure(self, array, mask, tmp_path): + import matplotlib.image as mpimg + + aplt.plot_array( + array=array, + output_path=str(tmp_path), + output_filename="without", + output_format="png", + ) + aplt.plot_array( + array=array, + mask=mask, + output_path=str(tmp_path), + output_filename="with", + output_format="png", + ) + + without = mpimg.imread(tmp_path / "without.png") + with_mask = mpimg.imread(tmp_path / "with.png") + + assert without.shape == with_mask.shape + differing = np.any(np.abs(without - with_mask) > 1.0e-6, axis=-1).sum() + assert differing > 0, "the mask outline was not drawn" + + def test__mask_is_optional(self, array, tmp_path): + aplt.plot_array( + array=array, + output_path=str(tmp_path), + output_filename="no_mask", + output_format="png", + ) + + assert (tmp_path / "no_mask.png").exists()