Feature/component colors #440
Conversation
WalkthroughAdds a centralized color-management subsystem (new color_processing and CONFIG.Plotting), exposes CalculationResults.setup_colors and propagation across segmented results, refactors plotting/aggregation/flow-system APIs to accept optional color inputs and resolve colors via process_colors, and updates examples and tests to use the new color workflow. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User / Script
participant R as CalculationResults
participant P as process_colors()
participant PL as Plotting layer
participant AG as Aggregation.plot
U->>R: call setup_colors(config? or path? , default_colorscale?)
activate R
R->>P: process_colors(colors_input, labels, default_colorscale)
activate P
P-->>R: label->color mapping
deactivate P
R-->>U: mapping stored in R.colors
deactivate R
U->>PL: call plot_heatmap(..., colors? )
activate PL
alt colors explicit
PL->>P: process_colors(colors, labels, CONFIG.Plotting.default_qualitative_colorscale)
P-->>PL: resolved mapping
else colors omitted
PL->>R: read R.colors
end
PL-->>U: render figure using resolved colors
deactivate PL
U->>AG: call Aggregation.plot(colormap?)
activate AG
AG->>P: process_colors(colormap or CONFIG.Plotting.default_qualitative_colorscale, labels)
P-->>AG: resolved colors list
AG-->>U: aggregated + raw figures using same colors
deactivate AG
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🧰 Additional context used🧬 Code graph analysis (1)flixopt/results.py (2)
⏰ 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)
🔇 Additional comments (6)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
flixopt/config.py (1)
236-252: Add missingPlottingconfiguration reset toreset()method.The
reset()method resetsLoggingandModelingconfigurations but omitsPlotting. Add a loop to resetPlottingattributes from_DEFAULTS['plotting']:for key, value in _DEFAULTS['plotting'].items(): setattr(cls.Plotting, key, value)Insert this block after the
Modelingreset loop and beforecls.config_name = _DEFAULTS['config_name'].
🧹 Nitpick comments (1)
flixopt/color_processing.py (1)
20-34: Consider edge case handling in RGB parsing.The function handles basic 'rgb(R, G, B)' to hex conversion but could be more robust:
- Doesn't handle 'rgba(R, G, B, A)' format (with alpha channel)
map(int, ...)on line 32 will raiseValueErrorif RGB values are non-integers or contain whitespace- No validation that RGB values are in valid range (0-255)
If needed, enhance robustness:
def _rgb_string_to_hex(color: str) -> str: """Convert Plotly RGB string format to hex. Args: color: Color in format 'rgb(R, G, B)' or already in hex Returns: Color in hex format '#RRGGBB' """ - if color.startswith('rgb('): + if color.startswith('rgb(') or color.startswith('rgba('): # Extract RGB values from 'rgb(R, G, B)' format - rgb_str = color[4:-1] # Remove 'rgb(' and ')' - r, g, b = map(int, rgb_str.split(',')) + start = 5 if color.startswith('rgba(') else 4 + rgb_str = color[start:-1].strip() + parts = [p.strip() for p in rgb_str.split(',')] + r, g, b = int(parts[0]), int(parts[1]), int(parts[2]) + # Clamp values to valid range + r, g, b = max(0, min(255, r)), max(0, min(255, g)), max(0, min(255, b)) return f'#{r:02x}{g:02x}{b:02x}' return color
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
examples/01_Simple/simple_example.py(1 hunks)examples/04_Scenarios/scenario_example.py(1 hunks)flixopt/aggregation.py(3 hunks)flixopt/color_processing.py(1 hunks)flixopt/config.py(3 hunks)flixopt/plotting.py(35 hunks)flixopt/results.py(18 hunks)tests/test_results_plots.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
examples/01_Simple/simple_example.py (1)
flixopt/results.py (1)
setup_colors(327-461)
examples/04_Scenarios/scenario_example.py (1)
flixopt/results.py (1)
setup_colors(327-461)
flixopt/aggregation.py (3)
flixopt/color_processing.py (1)
process_colors(37-105)flixopt/config.py (2)
CONFIG(72-377)Plotting(199-233)flixopt/plotting.py (1)
with_plotly(170-485)
flixopt/results.py (2)
flixopt/color_processing.py (1)
process_colors(37-105)flixopt/config.py (2)
CONFIG(72-377)Plotting(199-233)
flixopt/plotting.py (2)
flixopt/color_processing.py (1)
process_colors(37-105)flixopt/config.py (2)
CONFIG(72-377)Plotting(199-233)
🔇 Additional comments (28)
examples/01_Simple/simple_example.py (1)
115-117: LGTM! Clear demonstration of the color setup feature.The optional
setup_colors()call appropriately demonstrates the new centralized color handling capability for users. The comments clearly indicate this is optional and guide users on customization.tests/test_results_plots.py (2)
31-31: LGTM! Test updated to reflect new default colorscale.The change from 'viridis' to 'turbo' correctly aligns with the new
CONFIG.Plotting.default_qualitative_colorscale.
54-54: LGTM! Heatmap colorscale updated consistently.The 'turbo' colorscale aligns with the new plotting defaults and is consistent with the test fixture update.
examples/04_Scenarios/scenario_example.py (1)
199-206: LGTM! Excellent demonstration of custom color configuration.The
setup_colors()call showcases both direct component-to-color mappings and colorscale-to-components mappings, demonstrating the flexible API for advanced users.flixopt/config.py (3)
57-67: LGTM! Well-designed plotting defaults.The default values are sensible choices for visualization:
- 'turbo' provides perceptually uniform sequential colors
- 300 DPI ensures publication-quality exports
- 'plotly' as the default engine aligns with the framework's interactive visualization focus
199-233: LGTM! Comprehensive plotting configuration class.The
Plottingconfiguration class properly exposes all plotting defaults with appropriate type hints and helpful documentation.
369-376: LGTM! Plotting configuration properly serialized.The
to_dict()method correctly includes all exposedPlottingconfiguration attributes for serialization.flixopt/aggregation.py (3)
23-25: LGTM! Necessary imports for new color processing.The imports support the migration to centralized color handling via
process_colorsandCONFIG.
146-146: LGTM! Signature change enables flexible color configuration.Making
colormapoptional (withNoneas the default) allows the method to leverageCONFIG.Plotting.default_qualitative_colorscalewhen no explicit colormap is provided.
155-161: LGTM! Consistent color usage across both plots.The color processing correctly resolves colors once and reuses them for both original and aggregated data figures, ensuring visual consistency.
flixopt/color_processing.py (4)
37-105: LGTM! Well-structured color processing with clear cases.The function cleanly handles all four input formats (None, str, list, dict) with appropriate fallbacks and cycling behavior. The logic flow is clear and properly delegates to helper functions.
108-137: LGTM! Clean implementation of partial mapping fill.The function correctly identifies missing labels and fills them with colors from the default colorscale, with appropriate debug logging.
140-186: LGTM! Robust three-tier fallback strategy.The function provides excellent fallback handling:
- Try requested colorscale
- Fall back to specified fallback colorscale
- Ultimate fallback to basic 10-color palette
This ensures colors are always available even with invalid inputs.
189-236: LGTM! Comprehensive colorscale resolution across multiple sources.The function systematically tries Plotly qualitative, Plotly sequential, and Matplotlib colorscales with proper error handling and consistent hex format output. This provides broad compatibility with various colorscale naming conventions.
flixopt/plotting.py (5)
44-44: LGTM! Clean import of color processing module.The import follows the existing module structure and supports the centralized color processing workflow.
52-68: LGTM! Robust colorscale registration with version compatibility.The portland colorscale registration properly handles both modern (≥3.7) and legacy Matplotlib versions, with appropriate existence checks to avoid duplicate registration.
1199-1199: LGTM! Correct handling of sequential colorscale for heatmaps.The heatmap functions correctly:
- Use optional colors parameter with CONFIG fallback
- Apply
CONFIG.Plotting.default_sequential_colorscale(not qualitative)- Pass colors directly to
color_continuous_scalewithoutprocess_colors(appropriate for continuous data)This is the right approach for heatmap visualizations.
Also applies to: 1203-1203, 1283-1288
1418-1418: LGTM! Consistent sequential colorscale handling.Matplotlib heatmap correctly uses
CONFIG.Plotting.default_sequential_colorscaleand passes colors directly as thecmapparameter, matching the Plotly implementation.Also applies to: 1481-1483, 1536-1536
1565-1567: LGTM! Well-designed CONFIG integration with improved save/show logic.The changes provide excellent flexibility:
- Optional
showanddpiparameters default to CONFIG values- Explicit handling of save/show combinations prevents ambiguity
- Interactive backend detection for Matplotlib prevents errors in headless environments
- Clean separation between display and persistence concerns
Also applies to: 1585-1591, 1606-1632
flixopt/results.py (9)
18-19: LGTM! Clean imports for color processing functionality.The imports support the new centralized color management system.
259-259: LGTM! Central color storage initialized correctly.The
self.colorsdictionary provides a single source of truth for color mappings across all plots, enabling consistent visualization.
327-404: LGTM! Flexible config handling with good error messages.The
setup_colorsmethod provides excellent flexibility:
- Supports multiple config formats (dict, file, colorscale name)
- Gracefully handles None for default behavior
- Attempts both JSON and YAML parsing for files
- Falls back to treating strings as colorscale names if file doesn't exist
The type checking and error messages are clear and helpful.
406-446: Review process_colors argument order in setup_colors.At line 431:
colors_for_components = process_colors(colorscale_name, components)And at line 451:
default_colors = process_colors(default_colorscale, remaining_components)Based on the signature from
color_processing.py:def process_colors( colors: None | str | list[str] | dict[str, str], labels: list[str], default_colorscale: str = 'turbo', ) -> dict[str, str]:These calls are correct! The first argument is the color spec (colorscale name), second is the labels list.
However, there's a subtle issue: when
colorscale_nameis actually a colorscale name (str), andcomponentsis a list, this should work. But the return type isdict[str, str]mapping labels to colors. The code then doescomponent_colors.update(colors_for_components)which should work correctly.Actually, looking more carefully - this looks correct!
454-461: LGTM! Complete variable-to-color mapping generation.The final step correctly:
- Iterates through all components with their assigned colors
- Retrieves all variable names for each component (including flows)
- Assigns the component's color to all its variables
- Returns the complete mapping
This ensures color consistency across all variables belonging to a component.
1326-1327: LGTM! Well-designed color propagation pattern.The pattern
self._calculation_results.colors or colorsprovides excellent flexibility:
- Uses centrally configured colors from
setup_colors()when available- Falls back to explicitly passed
colorsparameter- Allows per-plot overrides while maintaining defaults
This design enables both consistency (via
setup_colors()) and flexibility (via explicit parameters).Also applies to: 1337-1337, 1494-1494, 1508-1508, 1744-1744, 1760-1760, 1800-1800
862-863: LGTM! Consistent optional parameter types across plotting methods.All plotting methods correctly updated to:
- Use
colors: plotting.ColorType | None = None- Use
show: bool | None = None- Use
facet_cols: int | None = NoneThis enables CONFIG-based defaults throughout the API while maintaining explicit override capability.
Also applies to: 868-868, 1167-1168, 1176-1176, 1357-1357, 1360-1360, 1623-1624, 1630-1630
2111-2117: LGTM! Consistent parameter updates in segmented and module-level functions.Both
SegmentedCalculationResults.plot_heatmapand the module-levelplot_heatmapcorrectly:
- Use optional
colors,show, andfacet_colsparameters- Maintain backward compatibility with deprecated parameters
- Delegate appropriately with correct parameter passing
Also applies to: 2242-2249
2180-2194: LGTM! Thorough deprecated parameter handling.The code properly:
- Checks for conflicts between deprecated and new parameters
- Raises clear ValueError when both are provided
- Issues DeprecationWarning with migration guidance
- Correctly migrates old parameter values to new ones
This ensures smooth transition for users while preventing ambiguous usage.
Also applies to: 2328-2342
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
flixopt/results.py (3)
1492-1503: Same precedence fix in pie chartsEnsure explicit
colorsoverridessetup_colors()here too.Apply:
- colors=self._calculation_results.colors or colors, + colors=colors if colors is not None else self._calculation_results.colors,Repeat for the Matplotlib branch.
Also applies to: 1505-1516
1740-1751: Same precedence fix inplot_charge_state(both traces and both engines)Honor an explicit
colorsarg.Apply in all three places:
- colors=self._calculation_results.colors or colors, + colors=colors if colors is not None else self._calculation_results.colors,Also applies to: 1756-1767, 1799-1806
1323-1334: Fix colors argument precedence in all plotting and comparison callsThe current logic (
self._calculation_results.colors or colors) incorrectly prioritizes preconfigured colors over explicitly provided arguments, contradicting the documented override behavior.Apply the fix to all 7 occurrences:
- colors=self._calculation_results.colors or colors, + colors=colors if colors is not None else self._calculation_results.colors,Locations:
- Line 1327 (with_plotly)
- Line 1338 (with_matplotlib)
- Line 1495 (with_comparison_data)
- Line 1509 (with_comparison_data pandas)
- Line 1745 (with_plotly)
- Line 1761 (with_plotly charge_state)
- Line 1801 (with_matplotlib)
🧹 Nitpick comments (3)
flixopt/results.py (2)
336-357: Doc/config mismatch: wildcards shown but not supportedTop-level class docs show patterns like
{'Solar*': 'Oranges'}, butsetup_colors()only accepts exact component names orcolorscale: [components]. Either remove wildcard examples or add simple glob support.Option A (docs): remove wildcard example.
Option B (code): acceptfnmatchin component validation.+import fnmatch @@ - for component in components: - if component not in self.components: - raise ValueError(f"Component '{component}' not found") + for component in components: + if component in self.components: + continue + # Allow simple wildcards + matches = [c for c in self.components if fnmatch.fnmatch(c, component)] + if not matches: + raise ValueError(f"Component '{component}' not found") + components.extend([m for m in matches if m not in components])If you prefer Option A, I can submit a doc-only patch.
379-405: Config file loading: branch by file suffix firstTrying JSON then YAML regardless of extension can yield confusing errors. Prefer loader based on suffix, fallback to the other if that fails.
I can provide a small helper
load_mapping_from_file(path)to encapsulate this.CHANGELOG.md (1)
55-72: Changelog clarifications (optional)
- Mention that explicit
colorsargs override configured colors in plots (once fixed).- Note rgba→hex conversion for Matplotlib compatibility in color processing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
CHANGELOG.md(2 hunks)flixopt/color_processing.py(1 hunks)flixopt/config.py(3 hunks)flixopt/results.py(21 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- flixopt/config.py
🧰 Additional context used
🧬 Code graph analysis (1)
flixopt/results.py (2)
flixopt/color_processing.py (1)
process_colors(36-104)flixopt/config.py (2)
CONFIG(71-376)Plotting(198-232)
⏰ 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). (1)
- GitHub Check: test (3.10)
Description
Add optional
setup_colors()method to CalculationResults, that creates a mapping of variable_names to colors, to achieve stable colors across plotsType of Change
Related Issues
Closes #(issue number)
Testing
Checklist
Summary by CodeRabbit
New Features
Chores
Tests