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
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ 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
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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.
| # Build color map and classify traces from base figure | ||
| # trace.name -> color, trace.name -> 'positive'|'negative'|'mixed'|'zero' | ||
| color_map: dict[str, str] = {} | ||
| class_map: dict[str, str] = {} | ||
|
|
||
| for i, trace in enumerate(fig.data): | ||
| # Get color | ||
| if hasattr(trace, 'line') and trace.line and trace.line.color: | ||
| color_map[trace.name] = trace.line.color | ||
| else: | ||
| color_map[trace.name] = default_colors[i % len(default_colors)] | ||
|
|
||
| # Classify based on y values | ||
| y_vals = trace.y | ||
| if y_vals is None or len(y_vals) == 0: | ||
| class_map[trace.name] = 'zero' | ||
| else: | ||
| y_arr = np.asarray(y_vals) | ||
| y_clean = y_arr[np.abs(y_arr) > 1e-9] | ||
| if len(y_clean) == 0: | ||
| class_map[trace.name] = 'zero' | ||
| else: | ||
| has_pos = np.any(y_clean > 0) | ||
| has_neg = np.any(y_clean < 0) | ||
| if has_pos and has_neg: | ||
| class_map[trace.name] = 'mixed' | ||
| elif has_neg: | ||
| class_map[trace.name] = 'negative' | ||
| else: | ||
| class_map[trace.name] = 'positive' | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's explore the repository structure and locate the file
find . -type f -name "statistics_accessor.py" | head -5Repository: flixOpt/flixopt
Length of output: 91
🏁 Script executed:
# Read the relevant file to see the _style_area_as_bar implementation
wc -l flixopt/statistics_accessor.pyRepository: flixOpt/flixopt
Length of output: 94
🏁 Script executed:
# Read lines around 182-212 to see the _style_area_as_bar method
sed -n '170,220p' flixopt/statistics_accessor.pyRepository: flixOpt/flixopt
Length of output: 1897
🏁 Script executed:
# Search for _iter_all_traces to understand its implementation
rg -n "_iter_all_traces" flixopt/Repository: flixOpt/flixopt
Length of output: 281
🏁 Script executed:
# Check lines 237-238 as mentioned in the review
sed -n '230,245p' flixopt/statistics_accessor.pyRepository: flixOpt/flixopt
Length of output: 678
🏁 Script executed:
# Search for frame handling or animation-related code in the file
rg -n "frames\|animation" flixopt/statistics_accessor.pyRepository: flixOpt/flixopt
Length of output: 41
🏁 Script executed:
# Get the _iter_all_traces function definition
sed -n '148,165p' flixopt/statistics_accessor.pyRepository: flixOpt/flixopt
Length of output: 579
🏁 Script executed:
# Get full context around _style_area_as_bar to understand the overall structure
sed -n '165,180p' flixopt/statistics_accessor.pyRepository: flixOpt/flixopt
Length of output: 705
🏁 Script executed:
# Check if this function is actually used to style traces in animation frames
sed -n '237,265p' flixopt/statistics_accessor.pyRepository: flixOpt/flixopt
Length of output: 1085
🏁 Script executed:
# Get the entire _style_area_as_bar function
sed -n '168,240p' flixopt/statistics_accessor.pyRepository: flixOpt/flixopt
Length of output: 2813
🏁 Script executed:
# Check if there are any uses of _style_area_as_bar with animation to understand the real-world scenario
rg -B 5 -A 5 "_style_area_as_bar" flixopt/statistics_accessor.py | grep -i "frame\|anim" -A 3 -B 3Repository: flixOpt/flixopt
Length of output: 532
🏁 Script executed:
# Look at how balance(), carrier_balance(), storage() use _style_area_as_bar and if they create animations
rg -n "def balance\|def carrier_balance\|def storage" flixopt/statistics_accessor.pyRepository: flixOpt/flixopt
Length of output: 41
🏁 Script executed:
# Check one of these methods to see if they create animations
sed -n '1000,1100p' flixopt/statistics_accessor.py | head -80Repository: flixOpt/flixopt
Length of output: 3320
🏁 Script executed:
# Search for animations in these methods more directly
rg -n "frames\s*=" flixopt/statistics_accessor.pyRepository: flixOpt/flixopt
Length of output: 41
🏁 Script executed:
# Look at balance() method
rg -n "def balance" flixopt/statistics_accessor.pyRepository: flixOpt/flixopt
Length of output: 80
🏁 Script executed:
# Get balance() to see if it uses animation
sed -n '2100,2200p' flixopt/statistics_accessor.pyRepository: flixOpt/flixopt
Length of output: 4422
🏁 Script executed:
# Check the balance() method fully
sed -n '1575,1700p' flixopt/statistics_accessor.pyRepository: flixOpt/flixopt
Length of output: 5104
🏁 Script executed:
# Search for animation_frame usage in the file
rg -n "animation_frame" flixopt/statistics_accessor.pyRepository: flixOpt/flixopt
Length of output: 945
🏁 Script executed:
# Check if balance() or similar methods pass animation_frame to plotly
sed -n '1575,1650p' flixopt/statistics_accessor.pyRepository: flixOpt/flixopt
Length of output: 3182
🏁 Script executed:
# Check how animation_frame flows through to plotly
sed -n '1640,1680p' flixopt/statistics_accessor.pyRepository: flixOpt/flixopt
Length of output: 1401
🏁 Script executed:
# Verify _apply_slot_defaults behavior with animation_frame
rg -n "_apply_slot_defaults" flixopt/statistics_accessor.py | head -5Repository: flixOpt/flixopt
Length of output: 377
🏁 Script executed:
# Check _apply_slot_defaults to understand the flow better
sed -n '71,100p' flixopt/statistics_accessor.pyRepository: flixOpt/flixopt
Length of output: 1073
Classification ignores animation frames, leading to incorrect stacking in time-varying plots.
_style_area_as_bar builds class_map only from fig.data (line 186), so animation frames with different sign patterns (e.g., a trace "Solar" positive in summer, zero in winter) are misclassified and stacked incorrectly. The docstring promises animation support but the implementation classifies only from the base figure.
Aggregate sign information across _iter_all_traces(fig) before building the classification map to ensure consistent stackgroup assignment across all frames.
🔧 Proposed fix (aggregate sign across all traces)
- for i, trace in enumerate(fig.data):
+ sign_map: dict[str, tuple[bool, bool]] = {}
+ for i, trace in enumerate(_iter_all_traces(fig)):
# Get color
- if hasattr(trace, 'line') and trace.line and trace.line.color:
- color_map[trace.name] = trace.line.color
- else:
- color_map[trace.name] = default_colors[i % len(default_colors)]
+ if trace.name not in color_map:
+ if hasattr(trace, 'line') and trace.line and trace.line.color:
+ color_map[trace.name] = trace.line.color
+ else:
+ color_map[trace.name] = default_colors[i % len(default_colors)]
# Classify based on y values
y_vals = trace.y
if y_vals is None or len(y_vals) == 0:
- class_map[trace.name] = 'zero'
+ sign_map.setdefault(trace.name, (False, False))
else:
y_arr = np.asarray(y_vals)
y_clean = y_arr[np.abs(y_arr) > 1e-9]
if len(y_clean) == 0:
- class_map[trace.name] = 'zero'
+ sign_map.setdefault(trace.name, (False, False))
else:
- has_pos = np.any(y_clean > 0)
- has_neg = np.any(y_clean < 0)
- if has_pos and has_neg:
- class_map[trace.name] = 'mixed'
- elif has_neg:
- class_map[trace.name] = 'negative'
- else:
- class_map[trace.name] = 'positive'
+ prev_pos, prev_neg = sign_map.get(trace.name, (False, False))
+ sign_map[trace.name] = (prev_pos or np.any(y_clean > 0), prev_neg or np.any(y_clean < 0))
+
+ for name, (has_pos, has_neg) in sign_map.items():
+ if has_pos and has_neg:
+ class_map[name] = 'mixed'
+ elif has_neg:
+ class_map[name] = 'negative'
+ elif has_pos:
+ class_map[name] = 'positive'
+ else:
+ class_map[name] = 'zero'Also applies to: 237-238
🤖 Prompt for AI Agents
In `@flixopt/statistics_accessor.py` around lines 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.
| # Layout settings for unified hover | ||
| fig.update_layout( | ||
| hovermode='x unified', | ||
| xaxis_showspikes=True, | ||
| xaxis_spikecolor='gray', | ||
| xaxis_spikethickness=1, | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the file and examine the context around lines 261-267
cat -n flixopt/statistics_accessor.py | sed -n '240,290p'Repository: flixOpt/flixopt
Length of output: 174
🏁 Script executed:
# Search for any subplot or facet configuration in the file
rg -n "make_subplots|facet_row|facet_col|secondary_y" flixopt/statistics_accessor.pyRepository: flixOpt/flixopt
Length of output: 174
🏁 Script executed:
# Look for the function containing this code to understand its full context
ast-grep --pattern $'def $_($_) {
$$$
fig.update_layout(
hovermode=$_,
$$$
)
$$$
}'Repository: flixOpt/flixopt
Length of output: 41
🌐 Web query:
Plotly xaxis_showspikes faceted plots multiple axes update_layout
💡 Result:
Short answer — two ways:
-
Globally for all facet/subplot x-axes:
- Use fig.update_xaxes(...). Example:
fig.update_xaxes(showspikes=True, spikemode="across+marker", spikecolor="black", spikesnap="cursor") - This sets spike behaviour for every x-axis at once. [1][2]
- Use fig.update_xaxes(...). Example:
-
Per-axis (or for Plotly‑Express facets which create xaxis, xaxis2, xaxis3...):
- Use layout keys or the per-axis updater. Examples:
- Named in update_layout: fig.update_layout(xaxis=dict(showspikes=True,...), xaxis2=dict(showspikes=True,...))
- Or iterate/update each axis: fig.for_each_xaxis(lambda ax: ax.update(showspikes=True, spikemode="across")) or use fig.update_xaxes(..., row=R, col=C) when you created the subplots with make_subplots. [1][3][4]
- Use layout keys or the per-axis updater. Examples:
Notes / gotchas:
- showspikes only takes effect when hovermode is compatible (closest/unified behaviour); control spikemode, spikesnap, spikecolor, spikethickness per-axis. [1]
- Plotly Express facets produce multiple x-axis objects named xaxis, xaxis2, ... so target them by name, row/col, or with for_each_xaxis. [2][4]
- There are some limitations reported when trying to draw a single spike line across independent subplots (shared/overlay behaviour); forum threads discuss workarounds. [3]
References:
- Axis reference (showspikes, spikemode, etc.). [1]
- Facet plots / how facets create axes. [2]
- Community discussion about shared spikes and tick-label interactions. [3]
- Examples / iterating axes (for_each_xaxis / update_xaxes with row/col). [4]
🌐 Web query:
Plotly update_layout vs update_xaxes xaxis2 xaxis3 spikes configuration
💡 Result:
Short answer — use update_xaxes to change x-axis spike properties for one or many x-axes programmatically; use update_layout (or the xaxis/xaxis2/xaxis3 dicts) to set properties for a specific named axis. Note: spike drawing depends on hovermode and Plotly’s subplot behavior (spikes across separate subplot axes can be limited).
How to apply spikes
-
Apply to all x-axes at once:
fig.update_xaxes(showspikes=True, spikemode="across+marker", spikecolor="red")
(update_xaxes updates every xaxis by default). [1][2] -
Apply to a specific named axis via layout dict:
fig.update_layout(xaxis2=dict(showspikes=True, spikemode="across+marker"))
or set xaxis3 similarly. update_layout sets layout.xaxis, layout.xaxis2, etc. [1] -
Target subsets programmatically (selector / row/col):
fig.update_xaxes(patch, selector=..., row=..., col=...) — useful with make_subplots to pick specific axes. [2]
Important notes / gotchas
- showspikes only takes effect when layout.hovermode = "closest" (or use hover modes that produce per-point hover). Set fig.update_layout(hovermode="closest") if needed. [1][3]
- A spike with spikemode="across" draws across the plot area for that axis, but when make_subplots creates multiple distinct x-axes (xaxis, xaxis2, xaxis3) you may not get a single unified vertical spike across all subplots unless traces are forced to share one axis (or you map traces to the same x-axis). This is a known limitation/behavior discussed in the Plotly issues/forums. [3][4]
References
- Plotly Python layout.xaxis / update_xaxes reference (showspikes, spikemode, spikecolor, etc.). [1]
- update_xaxes selection / row/col/selector usage. [2]
- Plotly forum / GitHub discussion about spikes across subplots and shared axes behavior. [3][4]
If you want, tell me your exact code (make_subplots vs single plot, which axes to target) and I’ll give the precise update_xaxes / update_layout snippet.
Apply spike settings consistently across all x-axes using update_xaxes().
In faceted plots, fig.update_layout(xaxis_showspikes=True, ...) only affects the primary x-axis (xaxis), not secondary axes (xaxis2, xaxis3, etc.). Use fig.update_xaxes() to apply spike settings to all x-axes uniformly.
Suggested adjustment
- fig.update_layout(
- hovermode='x unified',
- xaxis_showspikes=True,
- xaxis_spikecolor='gray',
- xaxis_spikethickness=1,
- )
+ fig.update_layout(hovermode='x unified')
+ fig.update_xaxes(showspikes=True, spikecolor='gray', spikethickness=1)🤖 Prompt for AI Agents
In `@flixopt/statistics_accessor.py` around lines 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.
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.