Claude/refactor plotting module s6 zq1#236
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors plotting in autoarray by introducing standalone “direct matplotlib” plotting functions (e.g. plot_array, plot_grid, plot_yx, plot_inversion_reconstruction) and updating multiple plotter classes to use these helpers instead of the existing MatPlot* / MatWrap pathways.
Changes:
- Added new standalone plotting functions under
autoarray/plot/plots/plus shared utilities for saving/config-driven figure sizing. - Refactored multiple plotter classes (
Array2DPlotter,Grid2DPlotter,YX1DPlotter,ImagingPlotterMeta,FitImagingPlotterMeta,InversionPlotter,MapperPlotter) to call the new functions and centralized some overlay/output helpers. - Re-exported the new plotting functions from
autoarray.plotfor public use.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| autoarray/structures/plot/structure_plotters.py | Adds shared overlay/output helpers and migrates structure plotters to direct-matplotlib functions. |
| autoarray/plot/plots/array.py | New standalone plot_array() implementation. |
| autoarray/plot/plots/grid.py | New standalone plot_grid() implementation. |
| autoarray/plot/plots/yx.py | New standalone plot_yx() implementation. |
| autoarray/plot/plots/inversion.py | New standalone inversion reconstruction plotting function. |
| autoarray/plot/plots/utils.py | New shared helpers for figsize, saving figures, applying extents. |
| autoarray/plot/plots/init.py | Exposes the new standalone plot functions/utilities. |
| autoarray/plot/init.py | Re-exports standalone plot functions at the top-level plotting namespace. |
| autoarray/inversion/plot/mapper_plotters.py | Migrates mapper plotting to the new inversion/array plotting functions. |
| autoarray/inversion/plot/inversion_plotters.py | Introduces _plot_array helper and migrates inversion plotting calls. |
| autoarray/fit/plot/fit_imaging_plotters.py | Introduces _plot_array helper and migrates fit-imaging plotting calls. |
| autoarray/dataset/plot/imaging_plotters.py | Introduces _plot_array helper and migrates imaging plotting calls. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
autoarray/plot/array.py
Outdated
| log10_min = 1.0e-4 | ||
|
|
||
| clipped = np.clip(array, log10_min, None) | ||
| norm = LogNorm(vmin=vmin or log10_min, vmax=vmax or clipped.max()) |
autoarray/plot/inversion.py
Outdated
Comment on lines
+147
to
+154
| ax.imshow( | ||
| pix_array.native.array, | ||
| cmap=colormap, | ||
| norm=norm, | ||
| extent=pix_array.geometry.extent, | ||
| aspect="auto", | ||
| origin="upper", | ||
| ) |
Comment on lines
+103
to
+108
| fmt_list = output.format_list | ||
| fmt = fmt_list[0] if fmt_list else "show" | ||
|
|
||
| filename = output.filename_from(auto_filename) | ||
|
|
||
| if fmt == "show": |
Comment on lines
+185
to
+192
| # --- output ---------------------------------------------------------------- | ||
| if owns_figure: | ||
| save_figure( | ||
| fig, | ||
| path=output_path or "", | ||
| filename=output_filename, | ||
| format=output_format, | ||
| ) |
autoarray/plot/inversion.py
Outdated
Comment on lines
+93
to
+99
| extent = mapper.extent_from(values=pixel_values, zoom_to_brightest=zoom_to_brightest) | ||
|
|
||
| if isinstance(mapper.interpolator, (InterpolatorRectangular, InterpolatorRectangularUniform)): | ||
| _plot_rectangular(ax, pixel_values, mapper, norm, colormap, extent) | ||
| elif isinstance(mapper.interpolator, (InterpolatorDelaunay, InterpolatorKNearestNeighbor)): | ||
| _plot_delaunay(ax, pixel_values, mapper, norm, colormap) | ||
|
|
autoarray/plot/plots/utils.py
Outdated
Comment on lines
+57
to
+65
| if path: | ||
| os.makedirs(path, exist_ok=True) | ||
| try: | ||
| fig.savefig( | ||
| os.path.join(path, f"{filename}.{format}"), | ||
| dpi=dpi, | ||
| bbox_inches="tight", | ||
| pad_inches=0.1, | ||
| ) |
| mask=_mask_edge_from(self.array, self.visuals_2d), | ||
| grid=_grid_from_visuals(self.visuals_2d), | ||
| positions=_positions_from_visuals(self.visuals_2d), | ||
| lines=_lines_from_visuals(self.visuals_2d), |
Comment on lines
185
to
188
| grid=np.array(grid_plot.array), | ||
| ax=ax, | ||
| lines=_lines_from_visuals(self.visuals_2d), | ||
| color_array=color_array, |
Comment on lines
224
to
252
| def figure_1d(self): | ||
| """ | ||
| Plots the plotter's y and x values in 1D. | ||
| """ | ||
|
|
||
| self.mat_plot_1d.plot_yx( | ||
| y=self.y, | ||
| x=self.x, | ||
| visuals_1d=self.visuals_1d, | ||
| auto_labels=self.auto_labels, | ||
| should_plot_grid=self.should_plot_grid, | ||
| should_plot_zero=self.should_plot_zero, | ||
| plot_axis_type_override=self.plot_axis_type, | ||
| **self.plot_yx_dict, | ||
| """Plot the y and x values as a 1D line.""" | ||
| y_arr = self.y.array if hasattr(self.y, "array") else np.array(self.y) | ||
| x_arr = self.x.array if hasattr(self.x, "array") else np.array(self.x) | ||
|
|
||
| is_sub = self.mat_plot_1d.is_for_subplot | ||
| ax = self.mat_plot_1d.setup_subplot() if is_sub else None | ||
|
|
||
| output_path, filename, fmt = _output_for_mat_plot( | ||
| self.mat_plot_1d, is_sub, self.auto_labels.filename or "yx" | ||
| ) | ||
|
|
||
| shaded = None | ||
| if self.visuals_1d is not None and self.visuals_1d.shaded_region is not None: | ||
| shaded = self.visuals_1d.shaded_region | ||
|
|
||
| plot_yx( | ||
| y=y_arr, | ||
| x=x_arr, | ||
| ax=ax, | ||
| shaded_region=shaded, | ||
| title=self.auto_labels.title or "", | ||
| xlabel=self.auto_labels.xlabel or "", | ||
| ylabel=self.auto_labels.ylabel or "", | ||
| plot_axis_type=self.plot_axis_type or "linear", | ||
| output_path=output_path, | ||
| output_filename=filename, | ||
| output_format=fmt, | ||
| ) |
Add test_autoarray/inversion/plot/files/ and test_autoarray/plot/files/ which are generated by running tests but were not previously ignored. https://claude.ai/code/session_01B9sVEV54XWCa2LJw1C8gvv
Contributor
|
@Jammy2211 I've opened a new pull request, #237, to work on those changes. Once the pull request is ready, I'll request review from you. |
Plotting regressions (introduced by PR A1-A3): 1. conftest.py: also patch matplotlib.figure.Figure.savefig so PlotPatch captures saves made via fig.savefig() (the new direct-matplotlib path). 2. utils.py save_figure(): add `structure` param; when format=="fits" delegate to structure.output_to_fits() instead of fig.savefig() (matplotlib does not support FITS as an output format). 3. array.py plot_array(): thread `structure` through to save_figure(). 4. structure_plotters.py: add _zoom_array() helper that applies Zoom2D when zoom_around_mask is set in config, matching the old MatPlot2D.plot_array behaviour. Apply it in Array2DPlotter.figure_2d(). 5. imaging_plotters.py / fit_imaging_plotters.py: import and apply _zoom_array in _plot_array(); pass structure=array to plot_array() for FITS output. 6. grid.py: replace removed ndarray.ptp() with np.ptp() for NumPy 2.0 compat. 7. inversion.py _plot_rectangular(): guard against pixel_values=None (old MatPlot2D code handled this implicitly). Optional dependencies: - Add numba and pynufft to [dev] extras in pyproject.toml so they are installed by pip install -e ".[dev]" and CI picks them up automatically. - Pin pynufft to latest release (2025.2.1) which works with scipy >= 1.12 (2022.2.2 used pinv2 which was removed in scipy 1.12). https://claude.ai/code/session_01B9sVEV54XWCa2LJw1C8gvv
Add jax.config.update("jax_enable_x64", True) at module level in conftest.py
so all tests run with float64 precision. This fixes the pre-existing failure
in test__curvature_matrix_via_psf_weighted_noise_two_methods_agree where
float32 rounding produced a max absolute error of ~0.008, exceeding the
1e-4 tolerance.
https://claude.ai/code/session_01B9sVEV54XWCa2LJw1C8gvv
Documents the 12-step plan to remove Visuals1D/Visuals2D and pass overlay objects directly to matplotlib plot functions. https://claude.ai/code/session_01B9sVEV54XWCa2LJw1C8gvv
- Delete autoarray/plot/visuals/ entirely (Visuals1D, Visuals2D, AbstractVisuals) - Remove Visuals imports/exports from __init__.py, MatPlot1D, MatPlot2D - Remove plot_yx method from MatPlot1D (now handled by standalone plot_yx) - Array2DPlotter, Grid2DPlotter, YX1DPlotter: accept overlay kwargs directly - ImagingPlotter, MapperPlotter, InversionPlotter: remove visuals params - Mask auto-derived from array.mask via _auto_mask_edge() helper - mesh_grid is a first-class constructor arg on MapperPlotter/InversionPlotter - Update all plotter tests to use new direct-kwarg API - All 792 tests pass https://claude.ai/code/session_01B9sVEV54XWCa2LJw1C8gvv
…t tracking - Remove MatPlot1D, MatPlot2D container objects entirely - Remove multi_plotters.py (MultiFigurePlotter, MultiYX1DPlotter) - Remove mat_wrap.yaml, mat_wrap_1d.yaml, mat_wrap_2d.yaml config files - Remove mat_plot/ module (abstract.py, one_d.py, two_d.py) - All wrapper defaults now hardcoded directly in wrapper classes - Only 6 user-configurable options kept in general.yaml under mat_plot: section - AbstractPlotter holds output, cmap, use_log10, title directly (no MatPlot objects) - subplot_dataset(), subplot_fit() etc. rewritten as explicit matplotlib using plt.subplots() - figure_* methods accept optional ax parameter for subplot panel reuse - Remove is_for_subplot attribute and set_for_subplot() from all wrappers - Update all tests to match new hardcoded defaults and remove is_for_subplot test cases https://claude.ai/code/session_01B9sVEV54XWCa2LJw1C8gvv
…aunayDrawer - Delete all trivial wrapper classes: Figure, Axis, YLabel, XLabel, Title, Text, Annotate, Legend, TickParams, ColorbarTickParams, YTicks, XTicks, Units (408 lines), GridScatter, GridPlot, GridErrorbar, ArrayOverlay, VectorYXQuiver, Fill, Contour, PatchOverlay, and all scatter/plot subclasses (~30 files, ~1800 lines) - Delete all 1D wrappers: YXPlot, YXScatter, AXVLine, FillBetween - Rewrite Cmap as standalone class (no AbstractMatWrap): direct attributes instead of config_dict, same norm_from/vmin_from/vmax_from/symmetric_cmap_from API - Rewrite Colorbar as minimal class: set(ax) and set_with_color_values(cmap, vals, ax) - Rewrite DelaunayDrawer without AbstractMatWrap: plain __init__ kwargs, no Units/ ColorbarTickParams dependency - Inline Contour logic as contours= parameter in plots/array.py - Simplify AbstractPlotter: title is now plain str, set_backend inlined - Fix inversion_plotters.py: cmap.kwargs["vmax"] -> cmap.vmax (new API) - Delete 23 wrap test files; update test_cmap, test_colorbar, test_delaunay_drawer, test_abstract_plotters to test behaviour not config loading https://claude.ai/code/session_01B9sVEV54XWCa2LJw1C8gvv
All Plotter classes (AbstractPlotter, Array2DPlotter, Grid2DPlotter, YX1DPlotter, ImagingPlotter, InterferometerPlotter, FitImagingPlotter, FitInterferometerPlotter, MapperPlotter, InversionPlotter and their Meta variants) are deleted. The new API is function-based and closer to raw matplotlib: - plot_array_2d / plot_grid_2d / plot_yx_1d — structure-level wrappers that handle autoarray → numpy extraction before calling plot_array / plot_grid / plot_yx. - subplot_imaging_dataset / subplot_interferometer_dataset / subplot_interferometer_dirty_images — standalone subplot functions. - subplot_fit_imaging / subplot_fit_interferometer / subplot_fit_interferometer_dirty_images — standalone fit subplots. - plot_mapper / plot_mapper_image / subplot_image_and_mapper — mapper plots. - subplot_of_mapper / subplot_mappings — inversion subplots. Helper utilities (auto_mask_edge, zoom_array, numpy_grid, numpy_lines, numpy_positions, subplot_save) are now public functions in autoarray/plot/plots/utils.py and exported via autoarray.plot. All 746 tests pass. https://claude.ai/code/session_01B9sVEV54XWCa2LJw1C8gvv
Contributor
There was a problem hiding this comment.
Pull request overview
This PR substantially restructures autoarray’s plotting layer by replacing the prior MatWrap/MatPlot/Visuals-based API with direct-matplotlib, standalone plotting functions, and updates tests/configuration accordingly.
Changes:
- Added standalone plot functions (
plot_array,plot_grid,plot_yx, inversion plotting) plus higher-level convenience wrappers likeplot_array_2d/subplot_*. - Removed large portions of the legacy plotting wrapper and plotter infrastructure (MatWrap 1D/2D objects, Visuals, MatPlot objects, and related tests).
- Updated test utilities/config (e.g. savefig patching, visualize config, gitignore paths, dev extras).
Reviewed changes
Copilot reviewed 122 out of 125 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
test_autoarray/plot/wrap/two_d/test_vector_yx_quiver.py |
Removes legacy VectorYXQuiver test (module removed). |
test_autoarray/plot/wrap/two_d/test_patcher.py |
Removes legacy PatchOverlay test (module removed). |
test_autoarray/plot/wrap/two_d/test_grid_scatter.py |
Removes legacy GridScatter tests (module removed). |
test_autoarray/plot/wrap/two_d/test_grid_plot.py |
Removes legacy GridPlot tests (module removed). |
test_autoarray/plot/wrap/two_d/test_grid_errorbar.py |
Removes legacy GridErrorbar tests (module removed). |
test_autoarray/plot/wrap/two_d/test_derived.py |
Removes tests for derived wrapper classes (classes removed). |
test_autoarray/plot/wrap/two_d/test_delaunay_drawer.py |
Updates DelaunayDrawer test to new API surface. |
test_autoarray/plot/wrap/two_d/test_contour.py |
Removes legacy Contour test (module removed). |
test_autoarray/plot/wrap/two_d/test_array_overlay.py |
Removes legacy ArrayOverlay test (module removed). |
test_autoarray/plot/wrap/one_d/test_yx_scatter.py |
Removes legacy YXScatter test (module removed). |
test_autoarray/plot/wrap/one_d/test_yx_plot.py |
Removes legacy YXPlot tests (module removed). |
test_autoarray/plot/wrap/one_d/test_fill_between.py |
Removes legacy FillBetween test (module removed). |
test_autoarray/plot/wrap/one_d/test_axvline.py |
Removes legacy AXVLine test (module removed). |
test_autoarray/plot/wrap/base/test_units.py |
Removes Units tests (class removed). |
test_autoarray/plot/wrap/base/test_title.py |
Removes Title tests (class removed). |
test_autoarray/plot/wrap/base/test_ticks.py |
Removes tick wrapper tests (wrappers removed). |
test_autoarray/plot/wrap/base/test_tickparams.py |
Removes TickParams tests (class removed). |
test_autoarray/plot/wrap/base/test_text.py |
Removes Text tests (class removed). |
test_autoarray/plot/wrap/base/test_legend.py |
Removes Legend tests (class removed). |
test_autoarray/plot/wrap/base/test_label.py |
Removes label wrapper tests (classes removed). |
test_autoarray/plot/wrap/base/test_figure.py |
Removes Figure wrapper tests (class removed). |
test_autoarray/plot/wrap/base/test_colorbar_tickparams.py |
Removes ColorbarTickParams tests (class removed). |
test_autoarray/plot/wrap/base/test_colorbar.py |
Updates Colorbar tests to new simplified Colorbar implementation. |
test_autoarray/plot/wrap/base/test_cmap.py |
Updates Cmap tests to new simplified Cmap implementation. |
test_autoarray/plot/wrap/base/test_axis.py |
Removes Axis wrapper tests (class removed). |
test_autoarray/plot/wrap/base/test_annotate.py |
Removes Annotate tests (class removed). |
test_autoarray/plot/wrap/base/test_abstract.py |
Removes AbstractMatWrap config loading tests (system removed). |
test_autoarray/plot/visuals/test_visuals.py |
Replaces tests with comments after Visuals removal. |
test_autoarray/plot/test_multi_plotters.py |
Replaces tests with comments after multi-plotter removal. |
test_autoarray/plot/test_abstract_plotters.py |
Removes tests for AbstractPlotter subplot handling (system removed). |
test_autoarray/plot/mat_plot/test_mat_plot.py |
Replaces tests with comments after MatPlot removal. |
test_autoarray/inversion/plot/test_mapper_plotters.py |
Updates mapper plotting tests to new plot_mapper / subplot_image_and_mapper functions. |
test_autoarray/inversion/plot/test_inversion_plotters.py |
Updates inversion plotting tests to new plot_array_2d / plot_mapper / subplot_* functions. |
test_autoarray/fit/plot/test_fit_imaging_plotters.py |
Updates FitImaging plotting tests to new standalone plot/subplot functions. |
test_autoarray/dataset/plot/test_interferometer_plotters.py |
Updates interferometer plotting tests to new plot/subplot functions. |
test_autoarray/dataset/plot/test_imaging_plotters.py |
Updates imaging plotting tests to new plot/subplot functions. |
test_autoarray/conftest.py |
Forces JAX x64 in tests and patches both pyplot.savefig and Figure.savefig. |
pyproject.toml |
Expands dev extras to include numba and pynufft==2022.2.2. |
autoarray/structures/plot/structure_plotters.py |
Removes legacy structure Plotter classes (Array2DPlotter/Grid2DPlotter/YX1DPlotter). |
autoarray/structures/plot/structure_plots.py |
Adds new top-level plot_array_2d / plot_grid_2d / plot_yx_1d convenience functions. |
autoarray/structures/plot/__init__.py |
Re-exports new structure plotting functions. |
autoarray/plot/wrap/two_d/vector_yx_quiver.py |
Removes legacy VectorYXQuiver wrapper. |
autoarray/plot/wrap/two_d/serial_prescan_plot.py |
Removes legacy SerialPrescanPlot wrapper. |
autoarray/plot/wrap/two_d/serial_overscan_plot.py |
Removes legacy SerialOverscanPlot wrapper. |
autoarray/plot/wrap/two_d/positions_scatter.py |
Removes legacy PositionsScatter wrapper. |
autoarray/plot/wrap/two_d/patch_overlay.py |
Removes legacy PatchOverlay wrapper. |
autoarray/plot/wrap/two_d/parallel_overscan_plot.py |
Removes legacy ParallelOverscanPlot wrapper. |
autoarray/plot/wrap/two_d/origin_scatter.py |
Removes legacy OriginScatter wrapper. |
autoarray/plot/wrap/two_d/mesh_grid_scatter.py |
Removes legacy MeshGridScatter wrapper. |
autoarray/plot/wrap/two_d/mask_scatter.py |
Removes legacy MaskScatter wrapper. |
autoarray/plot/wrap/two_d/index_scatter.py |
Removes legacy IndexScatter wrapper. |
autoarray/plot/wrap/two_d/index_plot.py |
Removes legacy IndexPlot wrapper. |
autoarray/plot/wrap/two_d/grid_scatter.py |
Removes legacy GridScatter wrapper implementation. |
autoarray/plot/wrap/two_d/grid_plot.py |
Removes legacy GridPlot wrapper implementation. |
autoarray/plot/wrap/two_d/grid_errorbar.py |
Removes legacy GridErrorbar wrapper implementation. |
autoarray/plot/wrap/two_d/fill.py |
Removes legacy Fill wrapper implementation. |
autoarray/plot/wrap/two_d/delaunay_drawer.py |
Replaces DelaunayDrawer with a simplified direct-matplotlib implementation. |
autoarray/plot/wrap/two_d/contour.py |
Removes legacy Contour wrapper implementation. |
autoarray/plot/wrap/two_d/border_scatter.py |
Removes legacy BorderScatter wrapper. |
autoarray/plot/wrap/two_d/array_overlay.py |
Removes legacy ArrayOverlay wrapper. |
autoarray/plot/wrap/two_d/abstract.py |
Removes legacy AbstractMatWrap2D base class. |
autoarray/plot/wrap/two_d/__init__.py |
Only re-exports DelaunayDrawer after wrapper removals. |
autoarray/plot/wrap/one_d/yx_scatter.py |
Removes legacy YXScatter wrapper. |
autoarray/plot/wrap/one_d/yx_plot.py |
Removes legacy YXPlot wrapper. |
autoarray/plot/wrap/one_d/fill_between.py |
Removes legacy FillBetween wrapper. |
autoarray/plot/wrap/one_d/avxline.py |
Removes legacy AXVLine wrapper. |
autoarray/plot/wrap/one_d/abstract.py |
Removes legacy AbstractMatWrap1D base class. |
autoarray/plot/wrap/one_d/__init__.py |
Removes 1D wrapper exports. |
autoarray/plot/wrap/base/units.py |
Removes Units wrapper. |
autoarray/plot/wrap/base/title.py |
Removes Title wrapper. |
autoarray/plot/wrap/base/tickparams.py |
Removes TickParams wrapper. |
autoarray/plot/wrap/base/text.py |
Removes Text wrapper. |
autoarray/plot/wrap/base/legend.py |
Removes Legend wrapper. |
autoarray/plot/wrap/base/label.py |
Removes XLabel/YLabel wrappers. |
autoarray/plot/wrap/base/figure.py |
Removes Figure wrapper. |
autoarray/plot/wrap/base/colorbar_tickparams.py |
Removes ColorbarTickParams wrapper. |
autoarray/plot/wrap/base/colorbar.py |
Replaces Colorbar with a simplified implementation (no config integration). |
autoarray/plot/wrap/base/cmap.py |
Replaces Cmap with a simplified implementation (partial config integration). |
autoarray/plot/wrap/base/axis.py |
Removes Axis wrapper. |
autoarray/plot/wrap/base/annotate.py |
Removes Annotate wrapper. |
autoarray/plot/wrap/base/abstract.py |
Removes legacy AbstractMatWrap and backend/config logic (moved elsewhere). |
autoarray/plot/wrap/base/__init__.py |
Limits base wrap exports to Output/Cmap/Colorbar. |
autoarray/plot/wrap/__init__.py |
Limits wrap exports to Output/Cmap/Colorbar/DelaunayDrawer. |
autoarray/plot/visuals/two_d.py |
Removes Visuals2D implementation. |
autoarray/plot/visuals/one_d.py |
Removes Visuals1D implementation. |
autoarray/plot/visuals/abstract.py |
Removes Visuals base merge behavior. |
autoarray/plot/plots/yx.py |
Adds new standalone 1D plotting function. |
autoarray/plot/plots/utils.py |
Adds shared utilities (saving, config-derived figsize, overlays conversion). |
autoarray/plot/plots/inversion.py |
Adds standalone inversion reconstruction plotting. |
autoarray/plot/plots/grid.py |
Adds standalone 2D grid plotting. |
autoarray/plot/plots/__init__.py |
Re-exports new plot functions/utilities. |
autoarray/plot/abstract_plotters.py |
Removes AbstractPlotter and subplot/multiplot utilities. |
autoarray/plot/__init__.py |
Rebuilds public autoarray.plot exports around the new standalone API and sets backend. |
autoarray/inversion/plot/mapper_plotters.py |
Removes MapperPlotter class. |
autoarray/inversion/plot/mapper_plots.py |
Adds mapper plotting functions and subplot helper. |
autoarray/inversion/plot/__init__.py |
Re-exports new inversion plotting functions. |
autoarray/fit/plot/fit_interferometer_plots.py |
Adds new FitInterferometer subplot functions. |
autoarray/fit/plot/fit_imaging_plots.py |
Adds new FitImaging subplot function. |
autoarray/fit/plot/__init__.py |
Re-exports new fit plotting functions. |
autoarray/dataset/plot/interferometer_plots.py |
Adds new dataset subplot functions for interferometer. |
autoarray/dataset/plot/imaging_plots.py |
Adds new dataset subplot function for imaging. |
autoarray/dataset/plot/__init__.py |
Re-exports new dataset plotting functions. |
autoarray/config/visualize/mat_wrap_2d.yaml |
Removes legacy 2D MatWrap config. |
autoarray/config/visualize/mat_wrap_1d.yaml |
Removes legacy 1D MatWrap config. |
autoarray/config/visualize/mat_wrap.yaml |
Removes legacy shared MatWrap config. |
autoarray/config/visualize/general.yaml |
Reworks visualization config; adds mat_plot section for defaults. |
.gitignore |
Ignores additional test plot output directories. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
autoarray/plot/__init__.py
Outdated
Comment on lines
+20
to
+69
| from autoarray.plot.wrap.base.output import Output | ||
| from autoarray.plot.wrap.base.cmap import Cmap | ||
| from autoarray.plot.wrap.base.colorbar import Colorbar | ||
| from autoarray.plot.wrap.two_d.delaunay_drawer import DelaunayDrawer | ||
|
|
||
| from autoarray.plot.auto_labels import AutoLabels | ||
|
|
||
| from autoarray.plot.plots import ( | ||
| plot_array, | ||
| plot_grid, | ||
| plot_yx, | ||
| plot_inversion_reconstruction, | ||
| apply_extent, | ||
| conf_figsize, | ||
| save_figure, | ||
| subplot_save, | ||
| auto_mask_edge, | ||
| zoom_array, | ||
| numpy_grid, | ||
| numpy_lines, | ||
| numpy_positions, | ||
| ) | ||
|
|
||
| from autoarray.structures.plot.structure_plots import ( | ||
| plot_array_2d, | ||
| plot_grid_2d, | ||
| plot_yx_1d, | ||
| ) | ||
|
|
||
| from autoarray.dataset.plot.imaging_plots import subplot_imaging_dataset | ||
| from autoarray.dataset.plot.interferometer_plots import ( | ||
| subplot_interferometer_dataset, | ||
| subplot_interferometer_dirty_images, | ||
| ) | ||
|
|
||
| from autoarray.fit.plot.fit_imaging_plots import subplot_fit_imaging | ||
| from autoarray.fit.plot.fit_interferometer_plots import ( | ||
| subplot_fit_interferometer, | ||
| subplot_fit_interferometer_dirty_images, | ||
| ) | ||
|
|
||
| from autoarray.inversion.plot.mapper_plots import ( | ||
| plot_mapper, | ||
| plot_mapper_image, | ||
| subplot_image_and_mapper, | ||
| ) | ||
| from autoarray.inversion.plot.inversion_plots import ( | ||
| subplot_of_mapper, | ||
| subplot_mappings, | ||
| ) |
There was a problem hiding this comment.
The PR description focuses on refactoring 2D plotting helpers within existing Plotter classes, but this change set appears to remove/replace large parts of the plotting API (e.g. MatPlot1D/2D, Visuals classes, MultiPlotters) and re-exports a new standalone-function-based API from autoarray.plot. Please confirm the PR description is updated to reflect this breaking change, or scope the implementation back to the described refactor.
autoarray/plot/array.py
Outdated
Comment on lines
+108
to
+109
| if array is None or np.all(array == 0): | ||
| return |
There was a problem hiding this comment.
plot_array returns early when np.all(array == 0), which prevents plotting (and file output) for valid all-zero arrays (e.g. residual maps). This is a behavioral change from the previous plotting stack and can cause missing outputs; consider removing this guard (or only guarding on array is None).
Comment on lines
+78
to
+85
| ax.tripcolor( | ||
| source_pixelization_grid.array[:, 1], | ||
| source_pixelization_grid.array[:, 0], | ||
| simplices, | ||
| facecolors=facecolors, | ||
| edgecolors="None", | ||
| cmap=cmap_obj, | ||
| vmin=vmin, |
There was a problem hiding this comment.
DelaunayDrawer.__init__ accepts edgecolor, but ax.tripcolor(..., edgecolors="None") hard-codes no edges, so the edgecolor argument has no effect. Use the configured self.edgecolor (and consider whether edges should be disabled by default via linewidth/alpha instead).
Comment on lines
+1
to
+2
| # MultiFigurePlotter and MultiYX1DPlotter have been removed. | ||
| # Users should write their own matplotlib code for multi-panel plots. |
There was a problem hiding this comment.
This test file no longer contains any runnable tests, so pytest will collect it but it won't assert anything. Consider deleting the file entirely (or converting it to documentation) to avoid silently reducing test coverage / giving the impression these behaviors are still tested.
Comment on lines
+1
to
+2
| # MatPlot1D and MatPlot2D have been removed. | ||
| # Configuration is now done via direct wrapper objects passed to plotters. |
There was a problem hiding this comment.
This test file is now just comments and provides no assertions, which reduces effective test coverage while leaving a seemingly active test module. Consider removing the file (or marking as skipped/xfail with a clear reason) so the test suite reflects what is actually covered.
Comment on lines
+1
to
+2
| # Visuals classes (Visuals1D, Visuals2D) have been removed. | ||
| # Overlay objects are now passed directly to Plotter constructors. |
There was a problem hiding this comment.
This module has been reduced to comments only, so the previous Visuals behavior is no longer exercised by the test suite. If Visuals were intentionally removed, consider deleting the file (or adding targeted tests for the new replacement API) to avoid a silent coverage gap.
autoarray/plot/array.py
Outdated
Comment on lines
+119
to
+126
| if use_log10: | ||
| try: | ||
| from autoconf 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) | ||
| norm = LogNorm(vmin=vmin or log10_min, vmax=vmax or clipped.max()) |
There was a problem hiding this comment.
In the log-scale branch, clipped.max() is not NaN-safe; if array contains NaNs, vmax can become NaN and break LogNorm / imshow. Use np.nanmax(clipped) (and consider np.nanmin/clipping for vmin) to match the NaN-tolerant behavior of the previous MatWrap implementation.
Comment on lines
+72
to
+73
| if array is None or np.all(array == 0): | ||
| return |
There was a problem hiding this comment.
plot_array_2d returns early when the input is all zeros (np.all(array == 0)), which can suppress legitimate plots and expected output files (e.g. zero-valued diagnostic arrays). Consider removing this check (and rely on array is None only).
autoarray/plot/yx.py
Outdated
Comment on lines
+78
to
+79
| if y is None or np.count_nonzero(y) == 0 or np.isnan(y).all(): | ||
| return |
There was a problem hiding this comment.
plot_yx returns early when np.count_nonzero(y) == 0, which skips plotting all-zero series. All-zero data can be a valid plot (and should still write the output file); consider removing the count_nonzero guard and only skipping when y is None or np.isnan(y).all() (or len(y) == 0).
…ll private _plot_* helpers plot_array, plot_grid, and plot_yx now accept autoarray objects directly: - plot_array: calls zoom_array, extracts .native.array / .geometry.extent, derives mask via auto_mask_edge, converts all overlay params (grid, positions, lines, border, origin, array_overlay) via numpy_* helpers - plot_grid: extracts .array from grid objects, converts lines via numpy_lines, preserves extent_with_buffer_from before numpy conversion - plot_yx: extracts .array, falls back to .grid_radial for default x Consequences: - All private _plot_fit_array / _plot_dataset_array / _plot_array / _plot_grid / _plot_yx helpers removed from every subplot file; callers now call the core functions directly - plot_mapper_image removed (was just plot_array with extraction, now redundant) - structure_plots.py reduced to three re-export aliases - symmetric_vmin_vmax moved to utils.py and exported publicly https://claude.ai/code/session_01B9sVEV54XWCa2LJw1C8gvv
Both are unused after the plotting refactor. DelaunayDrawer functionality is already covered by _plot_delaunay in plots/inversion.py. Colorbar was only consumed by DelaunayDrawer. https://claude.ai/code/session_01B9sVEV54XWCa2LJw1C8gvv
…lues utils Cmap was no longer used by any plot function after the refactor. symmetric_cmap_from (returns matplotlib Normalize centred on zero) and set_with_color_values (attaches a colorbar via ScalarMappable, used for Delaunay mapper) are added as standalone functions in plots/utils.py. manual_tick_values/manual_tick_labels removed — callers can configure colorbars directly via the returned colorbar object. https://claude.ai/code/session_01B9sVEV54XWCa2LJw1C8gvv
Unused after plotter class removal. https://claude.ai/code/session_01B9sVEV54XWCa2LJw1C8gvv
Move all files to the top-level autoarray/plot/ package:
plots/{array,grid,yx,inversion,utils}.py -> plot/
wrap/base/output.py -> plot/output.py
wrap/segmentdata.py -> plot/segmentdata.py
Update all internal and external imports accordingly.
Delete plots/ and wrap/ subdirectories entirely.
https://claude.ai/code/session_01B9sVEV54XWCa2LJw1C8gvv
- Expand one-liner docstrings in utils.py to full NumPy-style with Parameters / Returns sections for all conversion helpers and save utils - Add detailed docstrings to _plot_rectangular and _plot_delaunay in inversion.py - Add docstrings to undocumented Output methods: output_path_from, filename_from, savefig, to_figure_output_mode, format / format_list properties - Run black across all plot-related modules (14 files reformatted) https://claude.ai/code/session_01B9sVEV54XWCa2LJw1C8gvv
Moves the helper from autogalaxy into autoarray/plot/utils.py and re-exports it from autoarray/plot/__init__.py so it is accessible as aa.plot.plot_visibilities_1d. https://claude.ai/code/session_01B9sVEV54XWCa2LJw1C8gvv
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 123 out of 127 changed files in this pull request and generated 8 comments.
Comments suppressed due to low confidence (1)
autoarray/plot/output.py:101
output_path_fromusesif format in "show", which is a substring membership test (so values like "s" would incorrectly be treated as show). This should beif format == "show"(and similarly avoid string-membership checks for other format comparisons).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+86
to
+92
| # --- colour normalisation -------------------------------------------------- | ||
| if use_log10: | ||
| norm = LogNorm(vmin=vmin or 1e-4, vmax=vmax) | ||
| elif vmin is not None or vmax is not None: | ||
| norm = Normalize(vmin=vmin, vmax=vmax) | ||
| else: | ||
| norm = None |
There was a problem hiding this comment.
When use_log10 is True, LogNorm(vmax=vmax) is created even when vmax is None. This can lead to runtime errors or inconsistent autoscaling depending on Matplotlib version. Consider computing a finite default vmax from pixel_values (e.g. np.nanmax) similar to autoarray.plot.array.plot_array.
Comment on lines
+184
to
+188
| extent=pix_array.geometry.extent, | ||
| aspect="auto", | ||
| origin="upper", | ||
| ) | ||
| else: |
There was a problem hiding this comment.
Docstring says “Both paths add a colorbar”, but the InterpolatorRectangularUniform (imshow) branch does not attach one. Either add a colorbar in the imshow path or update the docstring so the behavior is accurate and consistent.
Comment on lines
+15
to
+37
| def plot_yx( | ||
| y, | ||
| x=None, | ||
| ax: Optional[plt.Axes] = None, | ||
| # --- errors / extras -------------------------------------------------------- | ||
| y_errors: Optional[np.ndarray] = None, | ||
| x_errors: Optional[np.ndarray] = None, | ||
| y_extra: Optional[np.ndarray] = None, | ||
| shaded_region: Optional[Tuple[np.ndarray, np.ndarray]] = None, | ||
| # --- cosmetics -------------------------------------------------------------- | ||
| title: str = "", | ||
| xlabel: str = "", | ||
| ylabel: str = "", | ||
| label: Optional[str] = None, | ||
| color: str = "b", | ||
| linestyle: str = "-", | ||
| plot_axis_type: str = "linear", | ||
| # --- figure control (used only when ax is None) ----------------------------- | ||
| figsize: Optional[Tuple[int, int]] = None, | ||
| output_path: Optional[str] = None, | ||
| output_filename: str = "yx", | ||
| output_format: str = "png", | ||
| ) -> None: |
There was a problem hiding this comment.
This new standalone plotting function has multiple branches (errorbar vs scatter vs log scales, shaded_region, autoarray object extraction) but there are no tests exercising them (the previous MatWrap/YXPlot tests were removed). Adding a small pytest module that calls plot_yx across these modes would prevent regressions.
autoarray/plot/array.py
Outdated
| except AttributeError: | ||
| array = np.asarray(array) | ||
|
|
||
| if array is None or np.all(array == 0): |
There was a problem hiding this comment.
plot_array returns early for arrays that are entirely zeros (np.all(array == 0)). Zero-valued images can be valid (e.g. residual maps, masks, test fixtures), so this silently skips producing output and can make callers think plotting succeeded when nothing was generated. Consider only guarding on array is None (or on empty size), and let zero-valued arrays render normally.
Suggested change
| if array is None or np.all(array == 0): | |
| if array is None or array.size == 0: |
Comment on lines
+85
to
+87
| # guard: nothing to draw | ||
| if y is None or np.count_nonzero(y) == 0 or np.isnan(y).all(): | ||
| return |
There was a problem hiding this comment.
The guard np.count_nonzero(y) == 0 treats an all-zero series as “nothing to draw” and returns early. All-zero data is still valid to plot (and is common in diagnostics), so this will silently skip output. Consider guarding only on y is None, len(y) == 0, or np.isnan(y).all().
Comment on lines
+15
to
+38
| def plot_grid( | ||
| grid, | ||
| ax: Optional[plt.Axes] = None, | ||
| # --- errors ----------------------------------------------------------------- | ||
| y_errors: Optional[np.ndarray] = None, | ||
| x_errors: Optional[np.ndarray] = None, | ||
| # --- overlays --------------------------------------------------------------- | ||
| lines=None, | ||
| color_array: Optional[np.ndarray] = None, | ||
| indexes: Optional[List] = None, | ||
| # --- cosmetics -------------------------------------------------------------- | ||
| title: str = "", | ||
| xlabel: str = 'x (")', | ||
| ylabel: str = 'y (")', | ||
| colormap: str = "jet", | ||
| buffer: float = 0.1, | ||
| extent: Optional[Tuple[float, float, float, float]] = None, | ||
| force_symmetric_extent: bool = True, | ||
| # --- figure control (used only when ax is None) ----------------------------- | ||
| figsize: Optional[Tuple[int, int]] = None, | ||
| output_path: Optional[str] = None, | ||
| output_filename: str = "grid", | ||
| output_format: str = "png", | ||
| ) -> None: |
There was a problem hiding this comment.
plot_grid introduces new behavior (extent auto-computation, symmetric extents, colored points with optional errorbars, index highlighting) but there are currently no tests directly covering these branches. Consider adding pytest coverage for representative inputs (plain grid, list-like grid, colored grid, with/without extent).
autoarray/plot/array.py
Outdated
| except Exception: | ||
| log10_min = 1.0e-4 | ||
| clipped = np.clip(array, log10_min, None) | ||
| norm = LogNorm(vmin=vmin or log10_min, vmax=vmax or clipped.max()) |
There was a problem hiding this comment.
In the log-scale branch, clipped.max() does not ignore NaNs, so arrays containing NaNs can yield vmax=nan and cause LogNorm/imshow to fail. Use np.nanmax(clipped) (and ensure vmin/vmax are finite) to match the previous MatWrap behavior and avoid runtime errors.
Suggested change
| norm = LogNorm(vmin=vmin or log10_min, vmax=vmax or clipped.max()) | |
| # Determine vmin/vmax for LogNorm, ensuring they are finite and valid. | |
| 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: | |
| # Use nanmax so NaNs in the data do not propagate into vmax. | |
| with np.errstate(all="ignore"): | |
| vmax_log = np.nanmax(clipped) | |
| # Fallback if vmax is still invalid or does not define a proper log range. | |
| if not np.isfinite(vmax_log) or vmax_log <= vmin_log: | |
| vmax_log = vmin_log * 10.0 | |
| norm = LogNorm(vmin=vmin_log, vmax=vmax_log) |
autoarray/plot/__init__.py
Outdated
| from autoconf import conf | ||
|
|
||
| backend = conf.get_matplotlib_backend() | ||
| if backend not in "default": |
There was a problem hiding this comment.
The backend check uses if backend not in "default", which is a substring test and will behave incorrectly for some values (e.g. backend="d" would be treated as default). This should be an equality check (e.g. backend != "default").
Suggested change
| if backend not in "default": | |
| if backend != "default": |
… formats - autoarray/plot/array.py: add module-level aliases _zoom_array_2d and _mask_edge_coords pointing at the imported zoom_array / auto_mask_edge helpers for use by downstream packages (e.g. autogalaxy) - autoarray/plot/utils.py: save_figure now accepts format as either a str or a list/tuple of strings; iterates over all formats so a single call can write png + pdf (or any combination) simultaneously https://claude.ai/code/session_01B9sVEV54XWCa2LJw1C8gvv
- __init__.py: use backend != "default" instead of substring test - output.py: use format == "show" instead of substring test - array.py: guard on array.size == 0 instead of np.all(array == 0) so zero-valued images (residuals, masks) still render - array.py: compute LogNorm vmax with np.nanmax to handle NaN-containing arrays; guard against non-finite / degenerate ranges - yx.py: remove np.count_nonzero(y) == 0 guard so all-zero series still plots; keep only None / empty / all-NaN guards - inversion.py: compute LogNorm vmax from pixel_values (np.nanmax) instead of passing None, matching array.py behaviour - inversion.py: add plt.colorbar to the InterpolatorRectangularUniform (imshow) branch so both rectangular paths consistently show a colorbar https://claude.ai/code/session_01B9sVEV54XWCa2LJw1C8gvv
Previously the plot functions used hardcoded font sizes (title=16, xlabel/ylabel=14, ticks=12) and conf_figsize read a non-existent path, so config values were never applied. - utils.py: add conf_mat_plot_fontsize() to read from visualize/general/mat_plot/<section>/fontsize - utils.py: add apply_labels() which calls set_title/set_xlabel/ set_ylabel/tick_params using config-driven font sizes; eliminates the duplicated 4-line label block in every plot function - utils.py: fix conf_figsize() to read from mat_plot/figure/figsize (the key that actually exists in the config); add _parse_figsize() helper to handle YAML tuple-as-string encoding "(7, 7)" - array.py, grid.py, yx.py, inversion.py: replace hardcoded label blocks with apply_labels() - __init__.py: export apply_labels and conf_mat_plot_fontsize https://claude.ai/code/session_01B9sVEV54XWCa2LJw1C8gvv
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request refactors the plotting logic for 2D arrays in the
autoarraycodebase, centralizing the plotting functionality into new_plot_arrayhelper methods across several plotter classes. This change improves code reuse, simplifies method signatures, and standardizes how arrays are visualized. Additionally, it removes extensive docstrings from public methods, likely to reduce redundancy and encourage reference to external documentation.The most important changes are:
Centralization and Standardization of Plotting Logic:
_plot_arrayhelper method inImagingPlotter,FitImagingPlotterMeta, andInversionPlotterto handle all 2D array plotting using theplot_arrayfunction and shared plotting utilities. This replaces repeated calls tomat_plot_2d.plot_arraythroughout the codebase. [1] [2] [3]Refactoring of Plotting Calls:
figures_2dand related methods to use the new_plot_arrayhelper, reducing duplicated code and ensuring consistent handling of visuals, labels, and output file naming. [1] [2] [3] [4] [5]Imports and Utility Usage:
plot_arrayand various structure plotting utilities (_mask_edge_from,_grid_from_visuals, etc.) in relevant files to support the new centralized plotting workflow. [1] [2] [3]Docstring Cleanup:
__init__,figures_2d, andsubplot) to streamline the code and potentially reduce maintenance overhead. [1] [2] [3] [4] [5] [6]These changes collectively modernize the plotting infrastructure, making it easier to maintain and extend.