Make combine_categories = false plot per-component series (#111) - #5
Open
PabloBotin wants to merge 1 commit into
Open
Make combine_categories = false plot per-component series (#111)#5PabloBotin wants to merge 1 commit into
combine_categories = false plot per-component series (#111)#5PabloBotin wants to merge 1 commit into
Conversation
…nna-Platform#111) The `combine_categories = false` branch of `_plot_powerdata!` assigned the `PowerData`'s raw `Dict{Symbol, DataFrame}` and handed it to `_plot_dataframe!`, which takes a `DataFrame` — so a documented keyword raised a `MethodError` on `plot_powerdata`, `plot_results`, and their `!` and `_plotly` variants. Concatenate the per-category frames into the single wide frame the backends expect, dropping the duplicate `DateTime` columns. Component names recur across categories — a battery appears under both the storage charge and discharge categories, and renewable units under both dispatch and curtailment — so qualify each column as `Category__Component` using `PowerAnalytics.COMPONENT_NAME_DELIMITER`. That mirrors `metric_selector_to_string`, which is how PowerAnalytics names columns when `compute_all` combines frames, and it is what makes the documented `label_fn` behavior work: `label_component` and `label_variable` split on the same delimiter. Categories are visited in sorted order, since `Dict` iteration order is not stable and would otherwise shuffle series between calls. The docstrings claimed this keyword defaults to `false` when the code defaults to `true`; correct the docs rather than the long-standing behavior.
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 Sienna-Platform#111.
The bug
_plot_powerdata!'scombine_categories = falsebranch assigneddata = powerdata.data— aDict{Symbol, DataFrame}— and handed it to_plot_dataframe!, which takes aDataFrame. Confirmed against real data:A documented public keyword was dead on arrival for
plot_powerdata,plot_powerdata!,plot_results,plot_results!and all_plotlyvariants.Alongside it, all four docstrings claimed
combine_categories::Bool = falsewhile the code defaults totrue— the documented default was the one that crashed.What
falsenow doesConcatenates the per-category frames into the single wide frame the backends expect, dropping the duplicate
DateTimecolumns (the time axis is supplied separately aspowerdata.time).Column naming follows the PowerAnalytics convention. This is not cosmetic — component names genuinely collide across categories. Measured on
get_generation_data(results_uc):So each column is qualified as
Category__Componentusing the exportedPowerAnalytics.COMPONENT_NAME_DELIMITER("__", defined inInfrastructureSystems/src/common.jl:45). That mirrorsPA.metric_selector_to_string(PowerAnalytics/src/metrics.jl:199-200) — howcompute_allnames columns when it combines frames — rather than inventing a PowerGraphics-local scheme.It also makes the documented
label_fnbehavior actually work. The docstrings already saidcombine_categories = falseis how you "see the effect oflabel_fnon the raw labels", but the underlying columns are bare component names with no__to split on. Qualified, they parse correctly:label_component→"Solitude",label_variable→"ActivePowerVariable".Categories are visited in sorted order —
Dictiteration order is not stable, and without this the series and legend would shuffle between identical calls.Scope
plot_powerdataandplot_resultsfamilies (both route through_plot_powerdata!)._plot_fuel!untouched.ext/untouched — both backends already consume a single wide frame, so the fix is entirely in the backend-agnostic core. Thecombine_categories = truepath and the code default are unchanged.Verification
Full suite 48/48, zero Error log events; formatter clean.
Directly exercised against real generation data — 16/16, and 9 adversarial probes:
combine_categories = falseon both backendsDateTimecolumnstest_battlabel_component/label_variableon a qualified name"Solitude"/"ActivePowerVariable"PowerDataafter flatteningDateTimeedge case; repeat call does not double-prefix)combine_categories = truetruePA.combine_categoriesDataFrame, no throwArgumentErrorKnown limitations
:Awith columnB__Cand key:A__Bwith columnCboth qualify toA__B__C;hcatthen raisesArgumentError: Duplicate variable names: :A__B__C. This is a loud failure, not a silent merge, andmakeuniquewas deliberately not used so a genuine ambiguity cannot be papered over. It is unreachable with real PowerAnalytics data, where component names contain no__— verified that the fixture's adjacentRenewableDispatch/RenewableDispatch__Curtailmentkeys stay distinct.New columns must have the same length as old columns) rather than a PowerGraphics-level guard.PA.hcat_timed_dfswould give a time-axis-specific message; not used here since all frames in aPowerDatacome from the same results and are structurally equal-length.label_fnrewrites them before they reach the backend. The series count is asserted end to end on the real plot object for both backends.