Migrate to the PowerAnalytics 1.0 metrics API - #145
Open
PabloBotin wants to merge 12 commits into
Open
Conversation
Pin the current fuel/demand data contract ahead of the PowerAnalytics metrics-API migration: category naming (In/Out split, Curtailment, slack display names), charging sign conventions, palette-first column ordering, demand column naming, time-window and filter_func kwargs, and per-backend series counts.
PowerAnalytics imports get_system from PowerSimulations, so the unexported PA.PSI alias is unnecessary. Also extend the missing-system error to mention loading results with populate_system = true.
The IS.Results path now computes Metrics.calc_load_forecast over the all_loads selector (grouped into a single column renamed to "Load" so palette and label behavior are unchanged); a user filter_func folds into the selector. Time windows (initial_time/horizon, also spelled start_time/len) are applied by local row slicing because compute rejects unknown kwargs and mishandles len on simulation results in PA 1.4. The PSY.System path stays on the old get_load_data API, which has no new-API equivalent. The dead isnothing guard on the aggregated demand frame is replaced by an isempty check that can actually fire.
Assemble the fuel stack from PowerAnalytics Metric/ComponentSelector primitives instead of get_generation_data/make_fuel_dictionary/ categorize_data/combine_categories, preserving the exact column set, order, names, and signs. Components are assigned to a single category by replaying the old first-match-wins priority over the per-rule subselectors (the independent new selectors would otherwise double-count, e.g. NG-CC vs NG-Steam); generators fall back variable -> forecast parameter -> PowerOutput aux; storage/sources split into '<category> In'/'<category> Out' with charging flipped negative; slacks keep their BALANCE_SLACKVARS display names; unmatched components go to 'Other' with an error log. Also fix the net-load overlay to actually include storage charging by passing the charging total as extra_load, update the test mapping yaml for the new parser's strict fuel enums, and pin both behaviors with new tests.
…ta(::PowerData) plot_results now owns its dict-of-DataFrames path (DateTime stripped per entry, time axis from the first entry) instead of constructing PowerAnalytics.PowerData. The plot_powerdata methods move to src/deprecated.jl as forwarding shims that warn about removal in a future breaking release. combine_categories = false no longer crashes: it plots one trace per stored column, and the docstrings now state the actual default (true).
The Weave report template's tables now use the PowerAnalytics metrics API (calc_active_power per fuel category, calc_system_load_forecast) instead of the deprecated get_generation_data/get_load_data accessors; the Services table is dropped since get_service_data has no metrics-API equivalent. Docstrings drop the never-functional plot_fuel 'variables' kwarg, document the storage/sources kwargs, and reference plot_results instead of the deprecated plot_powerdata. The public API reference gains a hand-written Deprecated section.
PabloBotin
marked this pull request as draft
July 28, 2026 19:53
- plot_demand no longer crashes when a load type has no results; missing
results skip to the "No load data found" path (now an ArgumentError)
- warn on the unsupported `variables` kwarg of plot_fuel instead of
silently ignoring it
- warn when a custom generator mapping yaml contains ext_category keys,
which the PowerAnalytics 1.0 selector parser cannot honor
- _combine_result_categories: unknown `names` entries raise an actionable
ArgumentError; Vector{Symbol} accepted for the deprecated powerdata path
- docstrings: aggregate scope (System path only), time-window kwargs and
aliases, aggregate-function return-shape contract
- _FuelRule stores type_name::Symbol to avoid per-supertype allocations
- test: old-vs-new numeric equivalence of fuel category traces
…names
Every plot function now takes `backend::PlottingBackend`, defaulting to
`CairoMakieBackend()`:
plot_fuel(res) # CairoMakie
plot_fuel(res; backend = PlotlyLightBackend()) # PlotlyLight
The backend was already modeled as a value in src/backends.jl, so encoding it in
the function name doubled the public API without buying any dispatch. The ten
`_plotly`-suffixed functions keep working but warn and forward. Passing both a
`_plotly` name and a `backend` key word raises an ArgumentError rather than
letting one silently win, since the two would disagree about the renderer.
Eight per-plot behaviors that were resolved twice, once in each recipe, are now
resolved once in call_plots.jl and handed to the recipes through _PlotOptions:
fill default, line width, line style, draw order, title sentinel, empty input,
save path, and palette selection.
User-visible changes:
- Both backends select from the whole palette returned by `load_palette`, so
more series get a distinct color before the cycle repeats. PlotlyLight
previously drew from a narrower set, so its default colors change.
- `_default_save_format` dispatches on the backend, making the PlotlyLight
default `html`. A shared hardcoded "png" tripped the extension-rewrite warning
on every default-path PlotlyLight save. An explicit `format` still wins.
- CairoMakie non-stacked draw order now matches PlotlyLight.
- WeaveExt = ["PlotlyLight", "Weave"] -> "Weave". Neither the extension nor
generic_report_template.jmd touches PlotlyLight, so the old trigger withheld
`report` from a CairoMakie-only user.
The backend stubs dispatch per concrete backend. With `backend` defaulting to
CairoMakie, a PlotlyLight-only user reached a stub telling them to run `using
PlotlyLight` when they already had; each stub now names its own package, and the
CairoMakie one names the key word that selects the other backend.
Two save-path defects go with it. `_resolve_save_file` is now the single place a
save path is decided, and it replaces spaces in the title with underscores as
every entry point on main already did; centralizing the path had dropped that
for `plot_dataframe` alone. `_plot_demand!` read `:save` without removing it
from the key words it forwarded, so one call saved the figure twice under two
different names; it now strips `:save`, `:title`, and `:set_display` like the
other wrappers.
Tests: plot_introspection.jl reads rendered marks back out of both libraries so
value assertions run against either backend; test_backend_parity.jl enforces the
parity contract; test_demand_semantics.jl and test_fuel_categories.jl pin the
demand sign and the fuel-rule specificity. Suite is at 368 pass / 0 fail.
Docs gain explanation/backend_parity.md and a Change Backends how-to; the
orphaned explanation/stub.md is removed. Personal notes are ignored through the
user-level git ignore rather than this repository's shared .gitignore.
PabloBotin
force-pushed
the
feature/migrate-to-poweranalytics-new-api
branch
from
July 29, 2026 22:32
d9d0097 to
b89dc10
Compare
Route the report template's Load table through the new public `get_demand_data` rather than `calc_system_load_forecast`, which reported the requested instead of the served demand and disagreed with `plot_demand` under controllable load formulations. Delegate `_combine_result_categories` to `PowerAnalytics.combine_categories` instead of reimplementing it, keeping only the actionable error on an unknown `names` entry. Move `seriescolor`, `column_labels`, `interval`, the scaled data matrix and the net-sign classification into `_PlotOptions`, so neither recipe derives them independently and the third spelling of the sign test disappears. Lowercase the extension in the PlotlyLight writer so `.HTML` is recognized rather than silently rewritten to a different path, and pin it with a test. Hoist the shared "Accepted Key Words" documentation into `_COMMON_PLOT_KWARGS` and interpolate it, replacing eight verbatim copies. Keep `_report_plot_fuel` as a forwarding shim: report templates copied from an earlier release call it positionally, so removing it would throw `UndefVarError` on their next `report`.
PabloBotin
force-pushed
the
feature/migrate-to-poweranalytics-new-api
branch
from
July 29, 2026 22:34
b89dc10 to
39be6fe
Compare
PabloBotin
marked this pull request as ready for review
July 29, 2026 22:35
There was a problem hiding this comment.
Pull request overview
This PR migrates PowerGraphics’ internal data access from PowerAnalytics’ deprecated pre-1.0 accessors to the 1.0 metrics/selectors API, while preserving the public plotting surface via deprecations and a new backend keyword. It also unifies CairoMakie/PlotlyLight behavior by centralizing plot option resolution in src/call_plots.jl, and adds a substantial test harness to enforce backend parity and numeric equivalence with the legacy API.
Changes:
- Migrates
plot_demand,plot_fuel, andplot_resultsinternals to the PowerAnalytics 1.0 metrics/selectors API and introducesget_demand_data. - Replaces
_plotly-suffixed public APIs with abackend = ...keyword, keeping the old names as deprecated shims. - Adds extensive regression and parity tests plus documentation describing backend guarantees and deliberate differences.
Reviewed changes
Copilot reviewed 25 out of 26 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/test_yamls/generator_mapping.yaml | Fixes fuel enum typos and documents stricter parsing. |
| test/test_yamls/generator_mapping_specificity.yaml | New fixture mapping to pin rule-specificity behavior. |
| test/test_yamls/generator_mapping_incomplete.yaml | New fixture mapping to assert “Other” routing and error logging. |
| test/test_plot_creation.jl | Updates plot-creation tests to exercise plot_results and deprecated forwarding. |
| test/test_fuel_stack_behavior.jl | New behavioral tests for fuel stack semantics and net-load overlay across backends. |
| test/test_fuel_categories.jl | New tests pinning mapping specificity recovery and correspondence validation. |
| test/test_demand_semantics.jl | New tests for demand sign/magnitude semantics and get_demand_data contract. |
| test/test_backend_parity.jl | New backend parity contract tests (deprecated shims, save formats, draw order, titles, palette, etc.). |
| test/runtests.jl | Includes shared plot introspection helpers for single-file test runs. |
| test/plot_introspection.jl | New backend-agnostic plot introspection helpers to write parity assertions once. |
| src/PowerGraphics.jl | Exports backend types, adds backend-specific “not loaded” stubs, and wires deprecations. |
| src/label_utils.jl | Updates docs/examples from plot_powerdata to plot_results. |
| src/deprecated.jl | Implements _plotly and plot_powerdata deprecation shims and warning behavior. |
| src/definitions.jl | Unifies palette selection across backends; simplifies fuel color matching dispatch. |
| src/call_plots.jl | Centralizes plot option resolution, window aliasing, metrics-API demand/fuel pipelines, and backend unification. |
| src/backends.jl | Introduces PlottingBackend value types and default save-format dispatch. |
| report_templates/generic_report_template.jmd | Updates report template to use metrics API and get_demand_data; removes old services table. |
| README.md | Documents backend keyword, deprecated _plotly names, and get_demand_data. |
| Project.toml | Adjusts Weave extension trigger to no longer depend on PlotlyLight. |
| ext/plotly_recipes.jl | Refactors PlotlyLight recipe to consume _PlotOptions; adds series-count support and case-insensitive HTML save. |
| ext/plot_recipes.jl | Refactors CairoMakie recipe to consume _PlotOptions; adds series-count support and updated HTML error hint. |
| docs/src/reference/public.md | Splits public docs into “current” vs “deprecated” sections via filters. |
| docs/src/how_to_guides/backends.md | Documents per-plot backend selection and deprecation guidance. |
| docs/src/explanation/stub.md | Removes placeholder explanation stub. |
| docs/src/explanation/backend_parity.md | Adds explicit backend parity contract documentation. |
| docs/make.jl | Adds the Backend Parity Contract page to the docs nav. |
Suppressed comments (1)
src/call_plots.jl:1540
_plot_fuel!computessave_filefrom the rawtitlevalue, so the legacy sentineltitle = " "for “no title” produces a saved filename like_.<ext>instead of the standard untitled name (dataframe.<ext>). Normalize the title before calling_resolve_save_file(matching_resolve_titlesemantics).
title = get(kwargs, :title, "Fuel")
stack = get(kwargs, :stack, true)
palette = get(kwargs, :palette, PALETTE)
save_file = _resolve_save_file(backend, title, kwargs)
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
907
to
910
| title = get(kwargs, :title, "") | ||
| set_display = get(kwargs, :set_display, true) | ||
| save_fig = get(kwargs, :save, nothing) | ||
| save_file = _resolve_save_file(backend, title, kwargs) | ||
|
|
kdayday
requested changes
Aug 26, 2026
kdayday
left a comment
kdayday
left a comment
Contributor
There was a problem hiding this comment.
This looks like a big improvement, thank you!
- There are at least 3 TODO upstream items referencing back to PA. Would suggest just resolving those now and removing the current workarounds here.
- I think we should also avoid continuing to propagate multiple
start_time/initial_timeandlen/horizonusages, to avoid ambiguity. Would suggest converging on current PA behavior,initial_timeonly andhorizonwhich can handle eitherPeriodorInt
From running plots:
3. Demand curve should default to end-use only, not including storage in. It has changed.
Before:
Now:
|
|
||
| `report` takes the same key word: `report(res, out_path, template; backend = PlotlyLightBackend())`. | ||
|
|
||
| !!! warning "Deprecated: the `_plotly` suffix" |
Contributor
There was a problem hiding this comment.
Suggest removing this note
| unavoidable consequence of what CairoMakie and PlotlyLight each can do. This page draws | ||
| that line explicitly, so that neither users nor maintainers have to guess which is which. | ||
|
|
||
| The distinction matters. When a divergence is undocumented, a bug fixed in one recipe |
Contributor
There was a problem hiding this comment.
This page has unnecessary detail (e.g., old git issue reference) and kind of reads as Claude's rationale for this PR. Suggest removing maintainer-specific detail unless highly needed. Could be simplified to focus on the 2 tables and move the page to a Reference page on backend differences, instead of Explanation
| plot_fuel(WEAVE_ARGS["results"]; backend = WEAVE_ARGS["backend"]) | ||
| ``` | ||
|
|
||
| # Tables |
Contributor
There was a problem hiding this comment.
This whole table section appears to be less useful after the PA API change. Suggest either removing the section entirely or replacing the code comments commentary with outputs formatted as appropriate based on the new PA (production cost, services, etc.)
| treated as an execution count), so local row slicing is the only way to | ||
| preserve the old windowing behavior. | ||
| """ | ||
| # TODO upstream: fix `compute` time-window key words in PowerAnalytics |
Contributor
There was a problem hiding this comment.
I think it would be preferable to just resolve this in PA than recreate here
| # `add_fixed_parameters!` for load types with no variable stored). The order is | ||
| # not cosmetic: under a controllable formulation (`PowerLoadInterruption`, | ||
| # `PowerLoadDispatch`) the variable is the *served* load, while the forecast | ||
| # parameter is the demand that was requested, and PowerSimulations stores that |
Contributor
There was a problem hiding this comment.
We should review this after get_multiplier_value's change for psy6
| - `initial_time::DateTime`: start at a time other than the results initial time (`start_time` is accepted as an alias) | ||
| - `filter_func::Function`: filter components included in the total | ||
| """ | ||
| function get_demand_data( |
Contributor
There was a problem hiding this comment.
If this is the only non-plot function exported from PG... should it live somewhere else? PowerSimulations.jl? Then the second one below would also have to move, maybe to PSY? Alternatively, do they need to be exported? Is internal appropriate?
| # variable → parameter → aux-variable fallback chain, and categories with no | ||
| # contributing component must vanish instead of producing all-zero columns. | ||
|
|
||
| # TODO upstream: PowerAnalytics has no built-in metrics for these entry types |
|
|
||
| const PALETTE = load_palette() | ||
|
|
||
| # Unused inside PowerGraphics since both backends default to the full palette; |
Contributor
There was a problem hiding this comment.
Took a quick look and didn't see it used in any downstream packages. If this isn't used, would suggest deprecating or removing to avoid the impression that this is the active default
|
|
||
| PowerGraphics._report_plot_fuel( | ||
| WEAVE_ARGS["backend"], | ||
| plot_fuel( |
Contributor
There was a problem hiding this comment.
Low priority, but this template is a bit sparse. Bar fuel plot is less interesting that fuel stack plot below. Maybe add in a custom fuel stack for Services below the energy fuel plot instead?
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
_time_window_indices only handled an integer horizon, so plot_demand(results; horizon = Hour(12)) failed with a MethodError while plot_demand(system; horizon = Hour(12)) worked, because the system path delegates to PowerAnalytics.get_load_data which converts a period using the time series resolution. Convert a period against the resolution of the results time axis so both paths select the same rows, and raise actionable errors when the resolution cannot be inferred or the horizon is not a whole multiple of it. Also drop the hard coded start_time from the unknown initial time message, which named a key word the caller may not have passed.
The plot time window accepted four spellings: initial_time/horizon from PowerSystems and the new PowerAnalytics, plus start_time/len from PowerSimulations. Converge on initial_time/horizon, matching the API the rest of the migration targets. The aliases only ever existed on this branch, so they are removed outright rather than deprecated: main documents initial_time/horizon only, and none of the deprecated shims name any of the four terms. Also cover the fuel plot time window, which shared _time_window_indices with the demand path but had no test of its own, and pin the period horizon on the system path, where PowerAnalytics rather than _horizon_steps performs the conversion.
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.
Closes #144.
PowerAnalytics deprecated its pre-1.0 accessors (
get_generation_data,get_load_data,get_service_data,categorize_data,PowerData, …) in favor of the 1.0Metric/ComponentSelector API — see the
"Old PowerAnalytics" notice
and tracking issue
PowerAnalytics.jl#28.
This moves PowerGraphics' internals across without breaking any public signature.
Why this is bigger than #144
Proving the migration correct meant comparing old and new API output per category, per
timestep — which is how the bugs below surfaced. That comparison has to hold on both
backends, and asserting it revealed the two
ext/recipes had drifted: each derived its owndefaults for fill, line width, draw order, title handling, empty input, save paths and
palette. Eight behaviors resolved twice, independently — the same drift that let the
bar-stacking fix in #140 land in one recipe and silently not the other.
Once those are resolved once in
src/call_plots.jland the recipes only draw, the backend ispurely a value, which
src/backends.jlalready modeled. The ten_plotly-suffixed functionswere then duplicating the public API without buying any dispatch, so they became a
backendkey word.
This does not split into two PRs cleanly — the demand sign fix, the
start_time/lenfix andthe fuel-categorization rewrite live in the same functions as the backend unification, and the
migration's regression tests are written through the backend-parity harness. The commits are
ordered so the migration can still be read on its own: the first seven are the migration, the
last two the backend key word and review fixes.
1. Migration to the metrics API
plot_demand,plot_fuelandplot_resultsare all on the new API.plot_resultsno longerconstructs a
PA.PowerData, andplot_powerdata(::PowerData)is deprecated to a shim.One new export,
get_demand_data, returning the demand dataplot_demanddraws with itsDateTimeaxis. Reading a single load metric does not give the same answer (see the sign bugbelow) and there was no public route to those numbers — the report template needed one, and
every other public function returns a plot object.
2.
backendis now a key wordThe
_plotlynames still work but warn and forward. Passing both a_plotlyname and abackendkey word raises anArgumentErrorrather than letting one silently win.Behavior users will notice:
load_palettepalette, so more series get adistinct color before the cycle repeats. PlotlyLight's default colors change.
html. A shared hardcoded"png"made everydefault-path save trip the extension-rewrite warning. An explicit
formatstill wins.WeaveExt = ["PlotlyLight", "Weave"] → "Weave". Neither the extension nor the reporttemplate touches PlotlyLight, so the old trigger withheld
reportfrom CairoMakie-onlyusers.
docs/src/explanation/backend_parity.mdrecords what the backends guarantee to renderidentically and where they deliberately differ;
test/test_backend_parity.jlenforces it.Bugs fixed
calc_load_forecastapplies an unconditional-1, but PowerSimulations stores the loadparameter with a formulation-dependent sign, so a mixed system partially cancelled —
measured −3604.307 against a true demand of +8853.873.
_demand_data(::IS.Results)nowresolves
(calc_active_power, calc_load_forecast)per concrete load type.calc_system_load_forecastdirectly, reporting adifferent number than
plot_demandfor the same results — the bug above, shipped in thetemplate. It now goes through
get_demand_data.plot_fuelnet-load line now includes storage charging / source input, as its commentalways claimed; the offset was computed and dropped.
plot_results(...; combine_categories = false)crashed with aMethodError. The docstringsalso claimed
falsewas the default when the code defaults totrue— the documenteddefault was the one that crashed.
plot_demand(sys; start_time, len)ignored those key words on theSystempath while thedocstring advertised them.
plot_demand(...; save = dir)wrote the figure twice under two different names.save_plot(p, "out.HTML")silently rewrote the path on PlotlyLight, which compared against".html"exactly where CairoMakie lowercases first.AG_BYPRODUCT,WOOD_WASTE_SOLIDS) that theold parser silently never matched.
Behavior notes
then fuel), so no component is double-counted. Specificity comes from
PA.parse_fuel_category, not from parsing display names.PowerAnalytics.computemishandles window key words on simulation results.
ext_categoryentries in custom mapping yamls are ignored by the new selector parser, witha
@warn.@errorinstead of one per component.Intentionally still on the old API
plot_demand(::PSY.System)(get_load_data(::System)has no new-API equivalent),no_datetimeon user-supplied DataFrames, and the deprecatedplot_powerdata(::PowerData)shims. PowerAnalytics still maintains the old API, so these keep working until a future
breaking release.
Testing
391 pass / 0 fail, with roughly 1,600 added lines under
test/.The migration is pinned by a numeric equivalence test against the still-exported old API,
covering every category across both UC and ED problems — storage In/Out, Curtailment,
Unserved Energy, Over Generation included — agreeing bit-identically
(
maxabsdiff = 0.0), not merely within tolerance.New test files:
test_backend_parity.jl(the parity contract, both backends compareddirectly),
test_demand_semantics.jl(the sign bug and per-formulation metric resolution),test_fuel_categories.jl(rule specificity and enum validation),test_fuel_stack_behavior.jl(stack composition and the net-load line), andplot_introspection.jl(backend-agnostic helpers so parity assertions are written once).The public API was also exercised end-to-end outside the harness against real UC and ED
results — backend selection, the deprecated names, save formats and paths, palette assignment
and demand sign — with figures rendered on both backends for inspection.
Upstream issues
Workarounds are marked
# TODO upstreamand filed against PowerAnalytics: brokencomputetime-window key words, stale
get_subselectorsexport, inconsistent missing-result errortypes, missing
calc_system_slack_down/forecast metrics, andparse_generator_categoriesreturning
nothing. Thecalc_load_forecastsign bug is still to be filed.