Uh oh!
There was an error while loading. Please reload this page.
Use area chart instead of bar chart for better performance - #577
Conversation
- Classifies traces by analyzing y-values: positive, negative, mixed, zero - Sets stackgroup='positive' or stackgroup='negative' for proper separate stacking - Mixed values shown as dashed lines (no fill) - Opaque fills, no line borders, hv line shape Performance: ┌────────────────────────┬───────┐ │ Method │ Time │ ├────────────────────────┼───────┤ │ .plotly.bar() + update │ 0.14s │ ├────────────────────────┼───────┤ │ .plotly.area() + style │ 0.10s │ ├────────────────────────┼───────┤ │ Speedup │ ~1.4x │ └────────────────────────┴───────┘
📝 WalkthroughWalkthroughAdded three internal helpers— Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
# Iterate over all traces (main + animation frames) def _iter_all_traces(fig: go.Figure): yield from fig.data for frame in getattr(fig, 'frames', []) or []: yield from frame.data # Apply unified hover styling (works with any plot type) def _apply_unified_hover(fig: go.Figure, unit: str = '', decimals: int = 1): # Sets: <b>name</b>: value unit # + hovermode='x unified' + spike lines Updated Methods ┌───────────────────┬──────────────────────────────────────────────┐ │ Method │ Changes │ ├───────────────────┼──────────────────────────────────────────────┤ │ balance() │ + _apply_unified_hover(fig, unit=unit_label) │ ├───────────────────┼──────────────────────────────────────────────┤ │ carrier_balance() │ + _apply_unified_hover(fig, unit=unit_label) │ ├───────────────────┼──────────────────────────────────────────────┤ │ storage() │ + _apply_unified_hover(fig, unit=unit_label) │ └───────────────────┴──────────────────────────────────────────────┘ Result - Hover format: <b>Solar</b>: 45.3 kW - Hovermode: x unified (single tooltip for all traces) - Spikes: Gray vertical line at cursor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@flixopt/statistics_accessor.py`:
- Around line 182-212: _style_area_as_bar currently builds class_map from
fig.data only, ignoring animation frames; fix by aggregating sign info across
all traces returned by _iter_all_traces(fig) when computing class_map so each
trace.name's sign classification (positive/negative/mixed/zero) considers every
frame. Replace the loop over fig.data for classification with a pass over
_iter_all_traces(fig) that collects/concatenates y values per trace.name (or
updates has_pos/has_neg flags incrementally), then compute class_map[trace.name]
from those aggregated flags; keep color_map logic unchanged (or still derive
from fig.data if desired). Ensure you reference the functions/variables
_style_area_as_bar, _iter_all_traces, class_map, color_map, and default_colors
when making the change.
- Around line 261-267: The current fig.update_layout call sets
xaxis_showspikes/xaxis_spikecolor/xaxis_spikethickness which only affects the
primary x-axis; instead call fig.update_xaxes(...) to apply showspikes,
spikecolor and spikethickness to all x-axes in faceted plots. Replace or
supplement the fig.update_layout(xaxis_showspikes=..., xaxis_spikecolor=...,
xaxis_spikethickness=...) usage with a single fig.update_xaxes(showspikes=True,
spikecolor='gray', spikethickness=1) call so every xaxis (xaxis, xaxis2, xaxis3,
...) receives the same spike settings.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
flixopt/statistics_accessor.py
🧰 Additional context used
🧬 Code graph analysis (1)
flixopt/statistics_accessor.py (2)
flixopt/comparison.py (2)
_apply_slot_defaults(29-43)storage(564-622)flixopt/clustering/base.py (1)
_apply_slot_defaults(36-44)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test (3.13)
- GitHub Check: test (3.11)
- GitHub Check: test (3.14)
- GitHub Check: test (3.12)
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
… aggregating sign info across all traces returned by _iter_all_traces(fig), including animation frames. The color_map is still derived from fig.data. The implementation uses a sign_flags dictionary to incrementally update has_pos/has_neg flags for each trace.name, then computes class_map from those aggregated flags. 2. _apply_unified_hover (lines 271-274): Replaced the fig.update_layout(xaxis_showspikes=..., ...) with a single fig.update_xaxes(showspikes=True, spikecolor='gray', spikethickness=1) call so spike settings apply to all x-axes (xaxis, xaxis2, xaxis3, ...) in faceted plots.
Uh oh!
There was an error while loading. Please reload this page.
Performance:
┌────────────────────────┬───────┐
│ Method │ Time │
├────────────────────────┼───────┤
│ .plotly.bar() + update │ 0.14s │
├────────────────────────┼───────┤
│ .plotly.area() + style │ 0.10s │
├────────────────────────┼───────┤
│ Speedup │ ~1.4x │
└────────────────────────┴───────┘
Description
Brief description of the changes in this PR.
Type of Change
Related Issues
Closes #(issue number)
Testing
Checklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.