From 7ce60ca4ba2525851234b7b468b23aef51b6873d Mon Sep 17 00:00:00 2001 From: Pablo Botin Date: Mon, 27 Jul 2026 17:11:37 -0600 Subject: [PATCH] feat: honor `width`/`height`/`scale` when saving plots (#91) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The docstring advertised `width`, `height`, and `scale` as `save_plot` keywords, but neither backend implemented them. CairoMakie's `save_plot` accepted `kwargs...` and discarded every one; PlotlyLight filtered them against an allowlist of PlotlyBase names it could not forward anyway. Implement all three. On CairoMakie `width`/`height` set the saved scene size and `scale` multiplies `px_per_unit` (png) and `pt_per_unit` (pdf/svg), so `scale = 1` reproduces the previous output exactly. On PlotlyLight `width`/`height` set the Plotly layout and `scale` warns and is ignored, since HTML has no rendering scale. Both backends restore the plot object afterwards, including when the write throws: `Makie.save` resizes the scene and never restores it, so passing a size through would otherwise permanently resize the caller's figure. Also drop `SUPPORTED_PLOTLY_SAVE_KWARGS` and stop forwarding keywords to `show(io, MIME("text/html"), plot)`, which accepts none — any allowlisted keyword raised a `MethodError`. --- ext/plot_recipes.jl | 46 +++++++++++++- ext/plotly_recipes.jl | 58 +++++++++++++----- src/call_plots.jl | 35 ++++++++--- test/test_plot_creation.jl | 122 +++++++++++++++++++++++++++++++++++++ 4 files changed, 235 insertions(+), 26 deletions(-) diff --git a/ext/plot_recipes.jl b/ext/plot_recipes.jl index 7b76f24..0a149c1 100644 --- a/ext/plot_recipes.jl +++ b/ext/plot_recipes.jl @@ -328,6 +328,22 @@ function PowerGraphics.save_plot(plot::CairoMakiePlot, filename::String; kwargs. ) end +_save_dimension(::Symbol, ::Nothing) = nothing +function _save_dimension(name::Symbol, value::Integer) + value > 0 || + throw(ArgumentError("`$name` must be a positive integer, got $value")) + return Int(value) +end +_save_dimension(name::Symbol, value) = + throw(ArgumentError("`$name` must be a positive integer, got $value")) + +_save_scale(::Nothing) = nothing +function _save_scale(value::Real) + value > 0 || throw(ArgumentError("`scale` must be a positive number, got $value")) + return float(value) +end +_save_scale(value) = throw(ArgumentError("`scale` must be a positive number, got $value")) + function PowerGraphics.save_plot( plot::CairoMakiePlot, filename::String, @@ -344,7 +360,35 @@ function PowerGraphics.save_plot( ), ) end - CairoMakie.save(filename, plot.figure) + width = _save_dimension(:width, get(kwargs, :width, nothing)) + height = _save_dimension(:height, get(kwargs, :height, nothing)) + scale = _save_scale(get(kwargs, :scale, nothing)) + + # Only the three save-time keywords are forwarded: the caller's kwargs are the + # full plot kwarg set, which `CairoMakie.save` would reject or misinterpret. + save_kwargs = Dict{Symbol, Any}() + if !isnothing(scale) + # Each format reads a different resolution knob — png uses `px_per_unit`, + # pdf uses `pt_per_unit`, svg uses `pt_per_unit / 0.75` — so scale both + # Makie defaults and let each format pick the one it cares about. + save_kwargs[:px_per_unit] = 2.0 * scale + save_kwargs[:pt_per_unit] = 0.75 * scale + end + + scene = plot.figure.scene + original_size = size(scene) + resized = !isnothing(width) || !isnothing(height) + if resized + save_kwargs[:size] = + (something(width, original_size[1]), something(height, original_size[2])) + end + try + CairoMakie.save(filename, plot.figure; save_kwargs...) + finally + # `CairoMakie.save` resizes the scene in place and never restores it, so + # without this the saved size would leak into the caller's figure. + resized && resize!(scene, original_size) + end @info "saved plot" filename return filename end diff --git a/ext/plotly_recipes.jl b/ext/plotly_recipes.jl index 304f0a3..3b3cf33 100644 --- a/ext/plotly_recipes.jl +++ b/ext/plotly_recipes.jl @@ -184,8 +184,37 @@ function PowerGraphics._dataframe_plots_internal( return plot end -const SUPPORTED_PLOTLY_SAVE_KWARGS = - [:autoplay, :post_script, :full_html, :animation_opts, :default_width, :default_height] +_save_dimension(::Symbol, ::Nothing) = nothing +function _save_dimension(name::Symbol, value::Integer) + value > 0 || + throw(ArgumentError("`$name` must be a positive integer, got $value")) + return Int(value) +end +_save_dimension(name::Symbol, value) = + throw(ArgumentError("`$name` must be a positive integer, got $value")) + +# Runs `f` with `layout.width`/`layout.height` set, then restores the layout exactly. +# `EasyConfig.Config` auto-vivifies on read — `layout.width` on a layout without a +# width *inserts* an empty `Config` that serializes as `"width":{}` — so every read +# is `haskey`-guarded and absent keys are removed again rather than reset. +function _with_layout_size(f::Function, plot::PlotlyLight.Plot, width, height) + original = Dict{Symbol, Any}( + k => plot.layout[k] for k in (:width, :height) if haskey(plot.layout, k) + ) + isnothing(width) || setproperty!(plot.layout, :width, width) + isnothing(height) || setproperty!(plot.layout, :height, height) + try + return f() + finally + for k in (:width, :height) + if haskey(original, k) + setproperty!(plot.layout, k, original[k]) + else + delete!(plot.layout, k) + end + end + end +end # Two-arg `save_plot` for PlotlyLight plots; inferred from the plot type so # callers can write `save_plot(p, "out.html")` and hit the right backend. @@ -199,27 +228,26 @@ function PowerGraphics.save_plot(plot::PlotlyLight.Plot, filename::String; kwarg end function PowerGraphics.save_plot( - plot, + plot::PlotlyLight.Plot, filename::String, backend::PowerGraphics.PlotlyLightBackend; kwargs..., ) - save_kwargs = - Dict{Symbol, Any}(((k, v) for (k, v) in kwargs if k in SUPPORTED_PLOTLY_SAVE_KWARGS)) - @info "saving plot" filename - if last(splitext(filename)) == ".html" - open(filename, "w") do io - show(io, MIME("text/html"), plot; save_kwargs...) - end - else + width = _save_dimension(:width, get(kwargs, :width, nothing)) + height = _save_dimension(:height, get(kwargs, :height, nothing)) + isnothing(get(kwargs, :scale, nothing)) || + @warn "`scale` is ignored for HTML output; it only applies to the CairoMakie backend." + if lowercase(last(splitext(filename))) != ".html" # PlotlyLight doesn't have built-in image export # Users need to save HTML and convert externally, or use PlotlyBase.jl @warn "PlotlyLight only supports HTML export. Saving as HTML instead." filename - html_filename = replace(filename, r"\.[^.]+$" => ".html") - open(html_filename, "w") do io - show(io, MIME("text/html"), plot; save_kwargs...) + filename = replace(filename, r"\.[^.]+$" => ".html") + end + @info "saving plot" filename + _with_layout_size(plot, width, height) do + open(filename, "w") do io + show(io, MIME("text/html"), plot) end - return html_filename end return filename end diff --git a/src/call_plots.jl b/src/call_plots.jl index ace2451..4134489 100644 --- a/src/call_plots.jl +++ b/src/call_plots.jl @@ -837,31 +837,46 @@ end end """ - save_plot(plot, filename) + save_plot(plot, filename; kwargs...) + save_plot(plot, filename, backend; kwargs...) -Saves a plot to the specified filename. The backend is chosen from the plot -object's type: CairoMakie plots dispatch to the CairoMakie writer (png/pdf/svg), -PlotlyLight plots dispatch to the PlotlyLight writer (html). +Saves a plot to the specified filename. In the two-argument form the backend is +chosen from the plot object's type: CairoMakie plots dispatch to the CairoMakie +writer (png/pdf/svg), PlotlyLight plots dispatch to the PlotlyLight writer (html). +The three-argument form takes the backend (`CairoMakieBackend()` or +`PlotlyLightBackend()`) explicitly. # Arguments - `plot`: plot object returned by a `plot_*` function - `filename::String` : path to save to +- `backend` : `CairoMakieBackend()` or `PlotlyLightBackend()`, when given explicitly # Example ```julia res = solve_op_problem!(OpProblem) plot = plot_fuel(res) -save_plot(plot, "my_plot.png") # CairoMakie +save_plot(plot, "my_plot.png"; width = 800, height = 600, scale = 2) # CairoMakie plot = plot_fuel_plotly(res) -save_plot(plot, "my_plot.html") # PlotlyLight +save_plot(plot, "my_plot.html"; width = 800, height = 600) # PlotlyLight ``` -# Accepted Key Words (PlotlyLight backend only; CairoMakie ignores them) -- `width::Union{Nothing,Int}=nothing` -- `height::Union{Nothing,Int}=nothing` -- `scale::Union{Nothing,Real}=nothing` +# Accepted Key Words + +- `width::Int`, `height::Int`: size of the saved output, in the backend's own units. + They affect only the written file — the plot object is left unchanged. CairoMakie + reads them as Makie scene units, which the default resolution renders at 2 px each + for png (`width = 800` writes a 1600 px wide file) and 0.75 pt each for pdf/svg, and + fills a missing dimension from the figure's current size. PlotlyLight writes them to + the layout as CSS pixels and leaves a missing dimension to Plotly's own default. +- `scale::Real`: multiplies CairoMakie's default output resolution — pixel dimensions + for png, page extent for pdf/svg. Being relative to those defaults, it overrides a + custom `px_per_unit`/`pt_per_unit` theme. CairoMakie only; ignored with a warning for + HTML. + +These keywords may also be passed to any `plot_*` function, which forwards them to +`save_plot` when saving with `save`. """ # The 2-arg `save_plot(plot, filename)` form is defined per-backend via type # dispatch — see `ext/plot_recipes.jl` (CairoMakie) and `ext/plotly_recipes.jl` diff --git a/test/test_plot_creation.jl b/test/test_plot_creation.jl index 69cf878..556dc99 100644 --- a/test/test_plot_creation.jl +++ b/test/test_plot_creation.jl @@ -1,5 +1,16 @@ file_path = TEST_OUTPUTS +# Width and height of a PNG, read straight out of the IHDR chunk (big-endian +# UInt32 at byte offsets 16 and 20), to avoid pulling in an image reader. +function png_size(filename::String) + return open(filename, "r") do io + seek(io, 16) + width = ntoh(read(io, UInt32)) + height = ntoh(read(io, UInt32)) + (Int(width), Int(height)) + end +end + function test_plots(file_path::String; backend_pkg::String = "cairomakie") # Select plot functions based on backend if backend_pkg == "cairomakie" @@ -377,6 +388,117 @@ function test_plots(file_path::String; backend_pkg::String = "cairomakie") cleanup && rm(out_path; recursive = true) end + @testset "test $backend_pkg save sizing" begin + out_path = joinpath(file_path, backend_pkg * "_save_sizing") + !isdir(out_path) && mkdir(out_path) + p = plot_dataframe_fn( + gen_uc.data[:ActivePowerVariable__ThermalStandard], + gen_uc.time; + set_display = false, + title = "sizing", + stack = true, + ) + + if backend_pkg == "cairomakie" + original_size = size(p.figure.scene) + + default_png = joinpath(out_path, "default.png") + PG.save_plot(p, default_png) + # The 1280x720 default figure at Makie's default px_per_unit of 2. + @test png_size(default_png) == (2560, 1440) + + sized_png = joinpath(out_path, "sized.png") + PG.save_plot(p, sized_png; width = 800, height = 600) + @test png_size(sized_png) == (1600, 1200) + + scaled_png = joinpath(out_path, "scaled.png") + PG.save_plot(p, scaled_png; width = 800, height = 600, scale = 2) + @test png_size(scaled_png) == (3200, 2400) + + # Saving at a different size must never mutate the caller's figure. + @test size(p.figure.scene) == original_size + + # Save-time sizing is independent of the plot-time `size` kwarg, which + # `save_plot` must ignore even though `plot_*` forwards it here. + ignores_size = joinpath(out_path, "ignores_size.png") + PG.save_plot(p, ignores_size; size = (300, 200)) + @test png_size(ignores_size) == (2560, 1440) + + # `CairoMakie.save` resizes the scene before it writes, so a failure + # mid-write must still leave the figure at its original size. + @test_throws Exception PG.save_plot( + p, + joinpath(out_path, "missing_dir", "x.png"); + width = 321, + height = 123, + ) + @test size(p.figure.scene) == original_size + + for format in ("svg", "pdf") + vector_file = joinpath(out_path, "vector.$format") + PG.save_plot(p, vector_file; width = 400, height = 300, scale = 1.5) + @test filesize(vector_file) > 0 + end + @test size(p.figure.scene) == original_size + + @test_throws ArgumentError PG.save_plot(p, joinpath(out_path, "bad.html")) + @test_throws ArgumentError PG.save_plot( + p, + joinpath(out_path, "bad.png"); + width = 0, + ) + @test_throws ArgumentError PG.save_plot( + p, + joinpath(out_path, "bad.png"); + scale = -1, + ) + else + @test !haskey(p.layout, :width) + + sized_html = joinpath(out_path, "sized.html") + PG.save_plot(p, sized_html; width = 640, height = 480) + html = read(sized_html, String) + @test occursin("\"width\":640", html) + @test occursin("\"height\":480", html) + # An unguarded `layout.width` read would serialize as `"width":{}`. + @test !occursin("\"width\":{}", html) + # The layout must be left exactly as it was found. + @test !haskey(p.layout, :width) + @test !haskey(p.layout, :height) + + # Keywords `show` cannot accept must be dropped, not forwarded. + extra_html = joinpath(out_path, "extra.html") + PG.save_plot(p, extra_html; full_html = true) + @test filesize(extra_html) > 0 + + @test_logs (:warn, r"scale") match_mode = :any PG.save_plot( + p, + joinpath(out_path, "scaled.html"); + scale = 2, + ) + + # Non-HTML requests fall back to HTML and report the name written. + written = @test_logs (:warn, r"HTML") match_mode = :any PG.save_plot( + p, + joinpath(out_path, "raster.png"), + ) + @test written == joinpath(out_path, "raster.html") + @test isfile(written) + + # A failed write must still restore the layout it borrowed. + @test_throws Exception PG.save_plot( + p, + joinpath(out_path, "missing_dir", "x.html"); + width = 640, + height = 480, + ) + @test !haskey(p.layout, :width) + @test !haskey(p.layout, :height) + end + + cleanup && rm(out_path; recursive = true) + end + # HTML saving only works with PlotlyLight backend if backend_pkg == "plotlylight" @testset "test html saving" begin