Skip to content

Claude/replace plots cairomakie aj arg#131

Open
acostarelli wants to merge 13 commits intomainfrom
claude/replace-plots-cairomakie-AjArg
Open

Claude/replace plots cairomakie aj arg#131
acostarelli wants to merge 13 commits intomainfrom
claude/replace-plots-cairomakie-AjArg

Conversation

@acostarelli
Copy link
Collaborator

Replace Plots.jl with CairoMakie, and use package extensions for both plotting backends.

claude and others added 9 commits January 6, 2026 16:39
This commit replaces Plots.jl (with GR backend) with CairoMakie as the default
static plotting backend. PlotlyJS remains available for interactive plots.

Major changes:
- Add CairoMakie as a dependency, remove Plots.jl dependency
- Create new backend system (backends.jl) with CairoMakieBackend and PlotlyJSBackend
- Rewrite plot_recipes.jl to use CairoMakie instead of Plots.jl/GR
- Update plotly_recipes.jl to use PlotlyJS directly (not through Plots)
- Update all plotting functions in call_plots.jl to use new backend system
- Update color handling in definitions.jl for both backends
- Update make_report.jl to use new backend types
- Update tests to work with CairoMakie and PlotlyJS backends
- Export backend management functions: backend(), backend!(), cairomakie(), plotlyjs()

Benefits:
- Maintains two backends as requested: CairoMakie and PlotlyJS
- Simpler dependency tree without Plots.jl
- Improved performance with native CairoMakie rendering
- Cleaner architecture with explicit backend selection

Breaking changes:
- Default backend is now CairoMakie instead of GR
- Plots.jl is no longer re-exported
- Users should use PowerGraphics.backend!() instead of Plots.backend!()
- Plot objects are now CairoMakiePlot or PlotlyJS.Plot instead of Plots.Plot
This commit removes the global backend state system and makes the API
more predictable and explicit.

Changes:
- Remove global backend state (backend(), backend!(), cairomakie(), plotlyjs())
- Create explicit _plotly variants for all plotting functions:
  - plot_demand_plotly(), plot_demand_plotly!()
  - plot_dataframe_plotly(), plot_dataframe_plotly!()
  - plot_powerdata_plotly(), plot_powerdata_plotly!()
  - plot_results_plotly(), plot_results_plotly!()
  - plot_fuel_plotly(), plot_fuel_plotly!()
- Default functions (without _plotly suffix) always use CairoMakie
- Update all internal functions to use explicit backend parameters
- Update tests to select appropriate function based on backend
- Remove backend switching from exports

Benefits:
- API is now completely predictable - function name determines backend
- No global state to manage
- Easier to understand which backend is being used
- More explicit and maintainable code

Usage:
```julia
using PowerGraphics

# Use CairoMakie (default)
plot_fuel(results)

# Use PlotlyJS explicitly
using PlotlyJS
plot_fuel_plotly(results)
```
This commit fixes several issues with the new explicit API:

Fixes:
- Fix save_plot() function that was calling removed backend() function
- Use multiple dispatch instead of isa() check for save_plot
- Update __init__() message to reflect new API (no more backend switching)
- Restrict HTML saving test to PlotlyJS backend only

Changes:
- save_plot() now has two methods:
  - save_plot(plot::CairoMakiePlot, ...) for CairoMakie plots
  - save_plot(plot, ...) for PlotlyJS plots (generic fallback)
- HTML format test only runs for PlotlyJS backend (CairoMakie doesn't support HTML)
- Updated PlotlyJS load message to mention plot_*_plotly() functions

This makes the code more idiomatic Julia using proper multiple dispatch
instead of runtime type checks.
This commit fixes the CairoMakie implementation to follow proper Makie
semantics rather than Plots.jl patterns.

Issues Fixed:
1. **Legend duplication**: Previously called axislegend() on every data
   addition, creating multiple legends. Now tracks legend state and
   removes old legend before creating new one.

2. **Legend positioning**: Changed from :rt (right-top inside axis) to
   placing legend in separate grid cell fig[1, 2]. This matches the
   original Plots.jl :outerright behavior where legend is outside the
   plot area.

3. **Title overwriting**: Added check to only set title if not default,
   preventing empty titles from overwriting meaningful ones.

CairoMakie Semantics Verified:
- ✓ Axis creation: Axis(fig[1, 1]) is correct
- ✓ Axis properties: Direct assignment (plot.axis.xlabel = ...) is correct
- ✓ Limits: Using ylims!(axis, ...) is correct
- ✓ Plotting functions: lines!, band!, barplot!, stairs! all correct
- ✓ Grid layout: Legend in fig[1, 2] follows Makie's grid system

Key Difference from Plots.jl:
- Plots.jl: Legend position via symbols like :outerright
- CairoMakie: Legend placed in grid cells, e.g., Legend(fig[1, 2], axis)

The implementation now properly follows CairoMakie/Makie.jl conventions.

References:
- Makie Axis docs: https://docs.makie.org/dev/reference/blocks/axis
- Makie Legend docs: https://docs.makie.org/dev/reference/blocks/legend
- Makie Layout tutorial: https://docs.makie.org/dev/tutorials/layout-tutorial
Replace Plots.jl patterns with proper CairoMakie conventions:
- Bar plots: Use stack parameter with stack groups instead of offset
- XTicks: Use tuple format (positions, labels) instead of array
- Stacked area plots: Properly separate stairs! and band! calls for stair option
- Grouped bar plots: Ensure xticks uses proper array format with collect()

These changes ensure the plotting code follows CairoMakie's API rather than
carrying over Plots.jl semantics.
Major changes:
- Replace PlotlyJS dependency with PlotlyLight in Project.toml
- Rename PlotlyJSBackend to PlotlyLightBackend throughout codebase
- Rewrite src/plotly_recipes.jl to use PlotlyLight.jl API
  - Use PlotlyLight.Config for trace configuration
  - Use PlotlyLight.Plot() for empty plots
  - Direct property assignment for layout (plot.layout.*)
  - Simplified trace creation with Config objects
- Update all references in tests, docs, and templates
- PlotlyLight only supports HTML export (warn for other formats)

PlotlyLight.jl is an ultra-lightweight interface to Plotly.js that provides:
- Faster time-to-first-plot
- Direct mapping to Plotly.js API
- More nimble and lightweight than PlotlyJS.jl
Major test fixes to support CairoMakie and PlotlyLight backends:

1. test_plot_creation.jl:
   - Add dynamic file extension based on backend (.html for PlotlyLight, .png for CairoMakie)
   - PlotlyLight only supports HTML export natively
   - Update all test assertions to use correct file extensions
   - Apply to all test sections: plot production, powerdata, demand, fuel, and alternate palettes

2. test_reports.jl:
   - Replace old Plots.jl backend system (Plots.gr(), Plots.plotlylight())
   - Use new backend types: PG.CairoMakieBackend(), PG.PlotlyLightBackend()
   - Change default from "gr" to "cairomakie"
   - Test both backends: cairomakie and plotlylight

These changes ensure tests correctly validate both backends with their respective capabilities.
- Updated Project.toml files in / and test/ to use same PlotyLight version, and removed PowerGraphics from test/ version.
- Removed _get_ylims function from plot_recipes in favor of Makie's built in reset_limits!
- Convert DateTime axes to Float64 in _dataframe_plots_internal because band plots don't support DateTime, and all plots on an Axis must have the same typed axes.
- Replaced stacked barplot functionality to allow for labels and legends.
- Manually updated Project.toml for package extensions.
- Refactored backend plotting code to ext/
- Renamed save_plot -> save_plot_plotly, consistent with naming conventions
- Removed CairoMakie and PlotlyLight imports from PowerGraphics. User must now improt whichever they are using.
@jd-lara jd-lara requested a review from daniel-thom February 10, 2026 18:14
@codecov
Copy link

codecov bot commented Feb 10, 2026

Codecov Report

❌ Patch coverage is 89.87730% with 33 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.23%. Comparing base (e9ed582) to head (e0ccff7).

Files with missing lines Patch % Lines
src/call_plots.jl 79.09% 23 Missing ⚠️
ext/plot_recipes.jl 93.93% 6 Missing ⚠️
ext/plotly_recipes.jl 97.67% 2 Missing ⚠️
src/PowerGraphics.jl 75.00% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #131      +/-   ##
==========================================
+ Coverage   86.54%   87.23%   +0.68%     
==========================================
  Files           6        8       +2     
  Lines         342      478     +136     
==========================================
+ Hits          296      417     +121     
- Misses         46       61      +15     
Flag Coverage Δ
unittests 87.23% <89.87%> (+0.68%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
ext/PlotlyLightExt.jl 100.00% <100.00%> (ø)
ext/make_report.jl 100.00% <100.00%> (ø)
src/backends.jl 100.00% <100.00%> (ø)
src/definitions.jl 85.07% <100.00%> (+5.70%) ⬆️
ext/plotly_recipes.jl 97.67% <97.67%> (ø)
src/PowerGraphics.jl 75.00% <75.00%> (-25.00%) ⬇️
ext/plot_recipes.jl 93.93% <93.93%> (ø)
src/call_plots.jl 80.00% <79.09%> (+4.16%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jd-lara jd-lara requested review from sabrilg-nrel and removed request for daniel-thom February 24, 2026 21:29
@sabrilg-nrel
Copy link

sabrilg-nrel commented Feb 26, 2026

Hi @acostarelli 👋

Thanks for your work on this PR!

I’m currently testing/reviewing it and noticed an issue when calling plot_powerdata that does not occur with the latest released version of PowerGraphics.

When running:

plot_powerdata(powerdata)

I get the following error:

ERROR: MethodError: no method matching _empty_plot(::PowerGraphics.CairoMakieBackend)
The function `_empty_plot` exists, but no method is defined for this combination of argument types.
Closest candidates are:
  _empty_plot()
   @ PowerGraphics ~/.julia/packages/PowerGraphics/L4qnD/src/call_plots.jl:1
Stacktrace:
[1] _empty_plot()
   @ PowerGraphics ~/.julia/packages/PowerGraphics/L4qnD/src/call_plots.jl:2
[2] plot_powerdata(powerdata::PowerAnalytics.PowerData; kwargs::@Kwargs{stack::Bool})
   @ PowerGraphics ~/.julia/packages/PowerGraphics/L4qnD/src/call_plots.jl:350
[3] top-level scope
   @ ~/Documents/GitHub/ar_pcm/code_sienna_operations/simulation.jl:214

Could it be that backend handling needs an additional method or fallback definition?

Please let me know if you’d like more context. Happy to help debug further .

@acostarelli
Copy link
Collaborator Author

@sabrilg-nrel Are you using CairoMakie in this file? The backends are now loaded as Package Extensions. If this is the error, then I should probably adds a warning that says no backends have been loaded.

@sabrilg-nrel
Copy link

Thanks @acostarelli. Yes, I wasn’t explicitly loading CairoMakie in that file. After adding using CairoMakie, it worked.

Since the backends are now loaded via package extensions, it might be helpful to add either:

  • a warning when no backend has been loaded, or
  • a short note in the README explaining that users need to explicitly load (and add) CairoMakie or another backend.

That would make the new behavior a bit clearer for users updating from the released version.

I’ll continue testing the new features with the simulations I’m running and will share any additional feedback along the way.

Comment on lines +52 to +53
"At least one must be included for PowerGraphics to function properly. Move either import above this one " *
"to suppress this warning."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[JuliaFormatter] reported by reviewdog 🐶

Suggested change
"At least one must be included for PowerGraphics to function properly. Move either import above this one " *
"to suppress this warning."
"At least one must be included for PowerGraphics to function properly. Move either import above this one " *
"to suppress this warning."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants