Plotting: add save plots option#418
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to add an option to save figures produced by Plotting helpers (e.g., propagation plots, 1D slices, spectrograms) to disk via new savename/savepath keyword arguments.
Changes:
- Add
savename/savepathkwargs to multiple plotting functions to enableplt.savefig(...). - Close figures after saving in some plotting helpers to reduce open-figure accumulation.
- Remove several “other stats” items from
Plotting.statsoutput.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| """ | ||
| stats(output; kwargs...) | ||
|
|
||
| Plot all statistics available in `output`. Additional `kwargs` are passed onto `plt.plot()` | ||
| """ | ||
| function stats(output; kwargs...) | ||
| function stats(output; savename=nothing, savepath=pwd(), kwargs...) |
There was a problem hiding this comment.
The docstring still advertises stats(output; kwargs...), but the function now takes savename and savepath. Please update the docstring signature/keyword-argument documentation so users discover the new save option from the generated docs.
| haskey(stats, "pressure") && push!( | ||
| fstats, (stats["pressure"], "Pressure (bar)")) | ||
| haskey(stats, "dz") && push!(fstats, (1e6*stats["dz"], "Stepsize (μm)")) | ||
| haskey(stats, "core_radius") && push!(fstats, (1e6*stats["core_radius"], "Core radius (μm)")) | ||
| haskey(stats, "zdw") && push!(fstats, (1e9*stats["zdw"], "ZDW (nm)")) | ||
| haskey(stats, "mode_reconstruction_error") && push!( | ||
| fstats, (stats["mode_reconstruction_error"], "Mode error")) | ||
| haskey(stats, "transverse_points") && push!( | ||
| fstats, (stats["transverse_points"], "Transverse grid points")) | ||
| haskey(stats, "transverse_integral_error_rel") && push!( | ||
| fstats, (stats["transverse_integral_error_rel"], "Transverse integral error (relative)")) | ||
|
|
||
|
|
There was a problem hiding this comment.
This PR is titled about adding a save option, but this hunk also removes plotting of mode_reconstruction_error, transverse_points, and transverse integral error stats (which are still produced by Stats). If this removal is intentional, it should be called out in the PR description/release notes; otherwise, consider restoring these plots.
| sfig | ||
| if savename !== nothing | ||
| plt.savefig(joinpath(savepath, savename)) | ||
| plt.close(sfig) | ||
| end |
There was a problem hiding this comment.
time_1D now always returns nothing because the last expression is the if savename !== nothing block. The preceding sfig line does not affect the return value. Please return sfig (and only then optionally save/close), so callers can still display or further modify the figure.
| sfig | |
| if savename !== nothing | |
| plt.savefig(joinpath(savepath, savename)) | |
| plt.close(sfig) | |
| end | |
| if savename !== nothing | |
| plt.savefig(joinpath(savepath, savename)) | |
| plt.close(sfig) | |
| end | |
| return sfig |
| if savename !== nothing | ||
| savename1 = savename*"_1.png" | ||
| plt.savefig(joinpath(savepath, savename1)) | ||
| plt.close(pfig) | ||
| end |
There was a problem hiding this comment.
pfig is only defined when length(pstats) > 0, but it is unconditionally referenced in the savename !== nothing block. If pstats is empty, calling stats(...; savename=...) will throw UndefVarError: pfig not defined. Guard the save/close with Npl > 0 (or isdefined) or initialize pfig = nothing and check before closing/saving.
| if savename !== nothing | ||
| savename2 = savename*"_2.png" | ||
| plt.savefig(joinpath(savepath, savename2)) | ||
| plt.close(ffig) | ||
| end |
There was a problem hiding this comment.
ffig is only created when length(fstats) > 0, but the save/close block runs whenever savename !== nothing. If fstats is empty this will throw UndefVarError: ffig not defined. Please only save/close when the corresponding figure was actually created.
| end | ||
| [pfig, ffig] | ||
|
|
||
| if savename !== nothing | ||
| savename2 = savename*"_2.png" | ||
| plt.savefig(joinpath(savepath, savename2)) | ||
| plt.close(ffig) | ||
| end | ||
| end |
There was a problem hiding this comment.
stats previously returned [pfig, ffig], but it now has no return value (the last evaluated expression is the if savename !== nothing block). This is a breaking API change and will also make REPL usage less convenient. Consider preserving the prior return value (e.g., return figures even if you saved/closed them, or return the saved filenames in addition) so existing callers keep working.
| fig | ||
| if savename !== nothing | ||
| plt.savefig(joinpath(savepath, savename)) | ||
| end |
There was a problem hiding this comment.
prop_2D now always returns nothing because the final expression in the function is the if savename !== nothing block; the standalone fig line is not the last expression. This breaks existing usage (e.g., REPL will no longer show the figure). Ensure the function returns fig (or the list of figures in multimode) regardless of whether saving is enabled.
| fig | |
| if savename !== nothing | |
| plt.savefig(joinpath(savepath, savename)) | |
| end | |
| if savename !== nothing | |
| plt.savefig(joinpath(savepath, savename)) | |
| end | |
| return fig |
| sfig | ||
| if savename !== nothing | ||
| plt.savefig(joinpath(savepath, savename)) | ||
| plt.close(sfig) | ||
| end |
There was a problem hiding this comment.
spec_1D now always returns nothing because the final expression is the if savename !== nothing block. This is a regression from returning the figure object. Please restructure so sfig is returned regardless of savename.
| sfig | |
| if savename !== nothing | |
| plt.savefig(joinpath(savepath, savename)) | |
| plt.close(sfig) | |
| end | |
| if savename !== nothing | |
| plt.savefig(joinpath(savepath, savename)) | |
| plt.close(sfig) | |
| end | |
| sfig |
| if savename !== nothing | ||
| plt.savefig(joinpath(savepath, savename)) | ||
| plt.close(fig) | ||
| end |
There was a problem hiding this comment.
spectrogram(t, Et, ...) now always returns nothing due to the if savename !== nothing block being the final expression. Please return fig regardless of saving, to preserve existing behavior.
| end | |
| end | |
| return fig |
| """ | ||
| prop_2D(output, specaxis=:f) | ||
|
|
||
| Make false-colour propagation plots for `output`, using spectral x-axis `specaxis` (see | ||
| [`getIω`](@ref)). For multimode simulations, create one figure for each mode plus one for | ||
| the sum of all modes. | ||
|
|
||
| # Keyword arguments | ||
| - `λrange::Tuple(Float64, Float64)` : x-axis limits for spectral plot (wavelength in metres) | ||
| - `trange::Tuple(Float64, Float64)` : x-axis limits for time-domain plot (time in seconds) | ||
| - `dBmin::Float64` : lower colour-scale limit for logarithmic spectral plot | ||
| - `resolution::Real` smooth the spectral energy density as defined by [`getIω`](@ref). | ||
| """ | ||
| function prop_2D(output, specaxis=:f; | ||
| trange=(-50e-15, 50e-15), bandpass=nothing, | ||
| λrange=(150e-9, 2000e-9), dBmin=-60, | ||
| resolution=nothing, modes=nothing, oversampling=4, | ||
| resolution=nothing, modes=nothing, oversampling=4, savename=nothing, savepath=pwd(), |
There was a problem hiding this comment.
The prop_2D docstring's signature/keyword-argument list doesn't mention the newly added savename/savepath options. Please update the docstring so the generated docs reflect the new API.
No description provided.