Skip to content

Conversation

@FBumann
Copy link
Member

@FBumann FBumann commented Sep 23, 2025

Description

This PR collects all changes for the v3.0.0 release.
The main changes are the addition of multi period and stochastic modeling support

Type of Change

  • Bug fix
  • New feature
  • Documentation update
  • Code refactoring

Related Issues

Closes #(issue number)

Testing

  • I have tested my changes
  • Existing tests still pass

Checklist

  • My code follows the project style
  • I have updated documentation if needed
  • I have added tests for new functionality (if applicable)

Summary by CodeRabbit

  • New Features

    • Multi-dimension modeling (time/period/scenario) with weights; scenario controls; NetCDF/JSON export, FlowResults with per-flow access; improved effect sharing syntax; balanced storage and new final-state storage options; enhanced plotting styles.
  • Breaking Changes

    • Many public API renames and signature changes (model/submodel, invest/on‑off, inputs/outputs lists); plotting parameter renamed to style; calculation modeling now returns the Calculation object.
  • Documentation

    • Major rework: mathematical notation, modeling patterns, scenarios, recipes, and new examples (scenarios, two‑stage).
  • Chores

    • Logging defaults adjusted; optional logger-name and console stream settings.

* Bugfix plot_node_balance_pie()

* Scenarios/fixes (#252)

* BUGFIX missing conversion to TimeSeries

* BUGFIX missing conversion to TimeSeries

* Bugfix node_balance with flow_hours: Negate correctly

* Scenarios/filter (#253)

* Add containts and startswith to filter_solution

* Scenarios/drop suffix (#251)

Drop suffixes in plots and add the option to drop suffixes to sanitize_dataset()

* Scenarios/bar plot (#254)

* Add stacked bar style to plotting methods
* Rename mode to style  (line, bar, area, ...)

* Bugfix plotting

* Fix example_calculation_types.py

* Scenarios/fixes (#255)

* Fix indexing issue with only one scenario

* Bugfix Cooling Tower

* Add option for balanced Storage Flows (equalize size of charging and discharging)

* Add option for balanced Storage Flows

* Change error to warning (non-fixed size with piecewise conversion AND fixed_flow_rate with OnOff)

* Bugfix in DataConverter

* BUGFIX: Typo (total_max/total_min in Effect)

* Bugfix in node_balance() (negating did not work when using flow_hours mode

* Scenarios/effects (#256)

* Add methods to track effect shares of components and Flows

* Add option to include flows when retrieving effects

* Add properties and methods to store effect results in a dataset

* Reorder methods

* Rename and improve docs

* Bugfix test class name

* Fix the Network algorithm to calculate the sum of parallel paths, and be independent on nr of nodes and complexity of the network

* Add tests for the newtork chaining and the results of effect shares

* Add methods to check for circular references

* Add test to check for circular references

* Update cycle checker to return the found cycles

* Add checks in results to confirm effects are computed correctly

* BUGFIX: Remove +1 from prior testing

* Add option for grouped bars to plotting.with_plotly() and make lines of stacked bar plots invisible

* Reconstruct FlowSystem in CalculationResults on demand. DEPRECATION in CalculationResults

* ruff check

* Bugfix: save flow_system data, not the flow_system

* Update tests

* Scenarios/datasets results (#257)

* Use dataarray instead of dataset

* Change effects dataset to dataarray and use nan when no share was found

* Add method for flow_rates dataset

* Add methods to get flow_rates and flow_hours as datasets

* Rename the dataarrays to the flow

* Preserve index order

* Improve filter_edges_dataset()

* Simplify _create_flow_rates_dataarray()

* Add dataset for sizes of Flows

* Extend results structure to contain flows AND start/end infos

* Add FlowResults Object

* BUGFIX:Typo in _ElementResults.constraints

* Add flows to results of Nodes

* Simplify dataarray creation and improve FlowResults

* Add nice docstrings

* Improve filtering of flow results

* Improve filtering of flow results. Add attribute of component

* Add big dataarray with all variables but indexed

* Revert "Add big dataarray with all variables but indexed"

This reverts commit 08cd8a1.

* Improve filtering method for coords filter and add error handling for restoring the flow system

* Remove unnecessary methods in results .from_json()

* Ensure consistent coord ordering in Effects dataarray

* Rename get_effects_per_component()

* Make effects_per_component() a dataset instead of a dataarray

* Improve backwards compatability

* ruff check

* ruff check

* Scenarios/deprecation (#258)

* Deprecate .active_timesteps

* Improve logger warning

* Starting release notes

* Bugfix in plausibility_check: Index 0

* Set bargap to 0 in stacked bars

* Ensure the size is always properly indexed in results.

* ruff check

* BUGFIX in extract data, that causes coords in linopy to be incorrect (scalar xarray.DataArrays)

* Improve yaml formatting for model documentation (#259)

* Make the size/capacity a TimeSeries (#260)

* Scenarios/plot network (#262)

* Catch bug in plot_network with 2D arrays

* Add plot_network() to test_io.py

* Update deploy-docs.yaml:
Run on Release publishing instead of creation
and
only run for stable releases (vx.y.z)

* Bugfix DataConverter and add tests (#263)

* Fix doc deployment to not publish on non stable releases

* Remove unused code

* Remove legend placing for better auto placing in plotly

* Fix plotly dependency

* Improve validation when adding new effects

* Moved release notes to CHANGELOG.md

* Try to add to_dataset to Elements

* Remove TimeSeries

* Remove TimeSeries

* Rename conversion method to pattern: to_...

* Move methods to FlowSystem

* Drop nan values across time dimension if present

* Allow lists of values to create DataArray

* Update resolving of FlowSystem

* Simplify TimeSeriesData

* Move TImeSeriesData to Structure and simplyfy to inherrit from xarray.DataArray

* Adjust IO

* Move TimeSeriesData back to core.py and fix Conversion

* Adjust IO to account for attrs of DataArrays in a Dataset

* Rename transforming and connection methods in FlowSystem

* Compacted IO methods

* Remove infos()

* remove from_dict() and to_dict()

* Update __str__ of Interface

* Improve str and repr

* Improve str and repr

* Add docstring

* Unify IO stuff in Interface class

* Improve test tu utilize __eq__ method

* Make Interface class more robust and improve exceptions

* Add option to copy Interfaces (And the FlowSystem)

* Make a copy of a FLowSytsem that gets reused in a second Calculation

* Remove test_timeseries.py

* Reorganizing Datatypes

* Remove TImeSeries and TimeSeriesCollection entirely

* Remove old method

* Add option to get structure with stats of dataarrays

* Change __str__ method

* Remove old methods

* remove old imports

* Add isel, sel and resample methods to FlowSystem

* Remove need for timeseries with extra timestep

* Simplify IO of FLowSystem

* Remove parameter timesteps from IO

* Improve Exceptions and Docstrings

* Improve isel sel and resample methods

* Change test

* Bugfix

* Improve

* Improve

* Add test for Storage Bounds

* Add test for Storage Bounds

* CHANGELOG.md

* ruff check

* Improve types

* CHANGELOG.md

* Bugfix in Storage

* Revert changes in example_calculation_types.py

* Revert changes in simple_example.py

* Add convenient access to Elements in FlowSystem

* Get Aggregated Calculation Working

* Segmented running with wrong results

* Use new persistent FLowSystem to create Calculations upfront

* Improve SegmentedCalcualtion

* Improve SegmentedCalcualtion

* Fix SegmentedResults IO

* ruff check

* Update example

* Updated logger essages to use .label_full instead of .label

* Re-add parameters. Use deprecation warning instead

* Update changelog

* Improve warning message

* Merge

* Merge

* Fit scenario weights to model coords when transforming

* Merge

* Removing logic between minimum, maximum and fixed size from InvestParameters

* Remove selected_timesteps

* Improve TypeHints

* New property on InvestParameters for min/max/fixed size

* Move logic for InvestParameters in Transmission to from Model to Interface

* Make transformation of data more hierarchical (Flows after Components)

* Add scenario validation

* Change Transmission to have a "balanced" attribute. Change Tests accordingly

* Improve index validations

* rename method in tests

* Update DataConverter

* Add DataFrame Support back

* Add copy() to DataConverter

* Update fit_to_model_coords to take a list of coords

* Make the DataConverter more universal by accepting a list of coords/dims

* Update DataConverter for n-d arrays

* Update DataConverter for n-d arrays

* Add extra tests for 3-dims

* Add FLowSystemDimension Type

* Revert some logic about the fit_to_model coords

* Adjust FLowSystem IO for scenarios

* BUGFIX: Raise Exception instead of logging

* Change usage of TimeSeriesData

* Adjust logic to handle non scalars

* Adjust logic to _resolve_dataarray_reference into separate method

* Update IO of FlowSystem

* Improve get_coords()

* Adjust FlowSystem init for correct IO

* Add scenario to sel and isel methods, and dont normalize scenario weights

* Improve scenario_weights_handling

* Add warning for not scaled weights

* Update test_scenarios.py

* Improve util method

* Add objective to solution dataset.

* Update handling of scenario_weights update tests

* Ruff check. Fix type hints

* Fix type hints and improve None handling

* Fix coords in AggregatedCalculation

* Improve Error Messages of DataConversion

* Allow multi dim data conversion and broadcasting by length

* Improve DataConverter to handle multi-dim arrays

* Rename methods and remove unused code

* Improve DataConverter by better splitting handling per datatype. Series only matches index (for one dim). Numpy matches shape

* Add test for error handling

* Update scenario example

* Fix Handling of TimeSeriesData

* Improve DataConverter

* Fix resampling of the FlowSystem

* Improve Warning Message

* Add example that leverages resampling

* Add example that leverages resampling adn fixing of Investments

* Add flag to Calculation if its modeled

* Make flag for connected_and_transformed FLowSystem public

* Make Calcualtion Methods return themselfes to make them chainable

* Improve example

* Improve Unreleased CHANGELOG.md

* Add year coord to FlowSystem

* Improve dimension handling

* Change plotting to use an indexer instead

* Change plotting to use an indexer instead

* Use tuples to set dimensions in Models

* Bugfix in validation logic and test

* Improve Errors

* Improve weights handling and rescaling if None

* Fix typehint

* Update Broadcasting in Storage Bounds and improve type hints

* Make .get_model_coords() return an actual xr.Coordinates Object

* Improve get_coords()

* Rename SystemModel to FlowSystemModel

* First steps

* Improve Feature Patterns

* Improve acess to variables via short names

* Improve

* Add naming options to big_m_binary_bounds()

* Fix and improve FLowModeling with Investment

* Improve

* Tyring to improve the Methods for bounding variables in different scenarios

* Improve BoundingPatterns

* Improve BoundingPatterns

* Improve BoundingPatterns

* Fix duration Modeling

* Fix On + Size

* Fix InvestmentModel

* Fix Models

* Update constraint names in test

* Fix OnOffModel for multiple Flows

* Update constraint names in tests

* Simplify

* Improve handling of vars/cons and models

* Revising the basic structure of a class Model

* Revising the basic structure of a class Model

* Simplify and focus more on own Model class

* Update tests

* Improve state computation in ModelingUtilities

* Improve handling of previous flowrates

* Imropove repr and submodel acess

* Update access pattern in tests

* Fix PiecewiseEffects and StorageModel

* Fix StorageModel and Remove PreventSimultaniousUseModel

* Fix Aggregation and SegmentedCalculation

* Update tests

* Loosen precision in tests

* Update test_on_hours_computation.py and some types

* Rename class Model to Submodel

* rename sub_model to submodel everywhere

* rename self.model to self.submodel everywhere

* Rename .model with .submodel if its only a submodel

* Rename .sub_models with .submodels

* Improve repr

* Improve repr

* Include def  do_modeling() into __init__() of models

* Make properties private

* Improve Inheritance of Models

* V3.0.0/plotting (#285)

* Use indexer to reliably plot solutions with and wihtout scenarios/years

* ruff check

* Improve typehints

* Update CHANGELOG.md

* Bugfix from renaming to .submodel

* Bugfix from renaming to .submodel

* Improve indexer in results plotting

* rename register_submodel() to .add_submodels() adn add SUbmodels collection class

* Add nice repr to FlowSystemModel and Submodel

* Bugfix .variables and .constraints

* Add type checks to modeling.py

* Improve assertion in tests

* Improve docstrings and register ElementModels directly in FlowSystemModel

* Improve __repr__()

* ruff check

* Use new method to compare sets in tests

* ruff check

* Update Contribute.md, some dependencies and add pre-commit

* Pre commit hook

* Run Pre-Commit Hook for the first time

* Fix link in README.md

* Update Effect name in tests to be 'costs' instead of 'Costs' Everywhere
Simplify testing by creating a Element Library

* Improve some of the modeling and coord handling

* Add tests with years and scenarios

* Update tests to run with multiple coords

* Fix Effects dataset computation in case of empty effects

* Update Test for multiple dims
Fix Dim order in scaled_bounds_with_state
Bugfix logic in .use_switch_on

* Fix test with multiple dims

* Fix test with multiple dims

* New test

* New test for previous flow_rates

* V3.0.0/main fit to model coords improve (#295)

* Change fit_to_model_coords to work with a Collection of dims

* Improve fit_to_model_coords

* Improve CHANGELOG.md
# Conflicts:
#	CHANGELOG.md
#	flixopt/network_app.py
# Conflicts:
#	CHANGELOG.md
#	flixopt/network_app.py
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 23, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Major v3.0.0 refactor: introduces FlowSystemModel/Submodel architecture, multi-dimension support (time/period/scenario), xarray-based TimeSeriesData/DataConverter, redesigned effects/shares API, dataset/NetCDF/JSON IO and FlowResults, many public API renames, plotting style param, logging/config changes, and widespread test/example/docs updates.

Changes

Cohort / File(s) Summary
Core architecture & submodels
flixopt/structure.py, flixopt/flow_system.py, flixopt/elements.py
Adds FlowSystemModel/Submodel/ElementModel/Submodels registry; Interface serialization (to_dataset/from_dataset/to_netcdf/to_json); FlowSystem inherits Interface and gains periods/scenarios/weights, connect_and_transform, fit_to_model_coords, create_model.
Modeling primitives & patterns
flixopt/modeling.py, flixopt/features.py
New ModelingUtilities, ModelingPrimitives, BoundingPatterns; moves Investment/OnOff/Piecewise to Submodel lifecycle methods and introduces _do_modeling/_create_variables_and_constraints/_add_effects.
Effects & sharing
flixopt/effects.py
Reworks Effect API to temporal/periodic domains; replaces specific_share_to_other_effects_* with share_from_temporal/share_from_periodic; adds conversion-path utilities and cycle detection; integrates with submodels.
Calculation, aggregation & results
flixopt/calculation.py, flixopt/aggregation.py, flixopt/results.py
Calculations now return Calculation objects from do_modeling/solve; Full/Aggregated/Segmented adjusted; FlowSystemModel used to create models; CalculationResults accepts flow_system_data xr.Dataset, adds FlowResults and flow-aware accessors.
Temporal data & conversion
flixopt/core.py, flixopt/io.py, flixopt/utils.py
Adds TimeSeriesData, DataConverter.to_dataarray, ConversionError, data-broadcast helpers; IO normalizes complex strings, preserves attrs in NetCDF/JSON; round_floats → round_nested_floats; removed legacy TimeSeries helpers.
Components, flows & converters
flixopt/components.py, flixopt/elements.py, flixopt/linear_converters.py, flixopt/interface.py
Component/Flow/Bus/Converter constructors and create_model now use FlowSystemModel/Submodel; TemporalDataUser types adopted; InvestParameters/OnOffParameters/Piecewise APIs extended/renamed; Storage/Transmission gains balanced/final-state logic; conversion factor sign semantics adjusted in some converters.
FlowSystem API & IO
flixopt/flow_system.py, flixopt/structure.py
FlowSystem adds periods/scenarios, validation helpers, resample/sel/isel, to_dataset/from_dataset/to_json, create_model(normalize_weights), fit_effects_to_model_coords and improved element access/repr.
Plotting & examples
flixopt/plotting.py, examples/*
modestyle for plotting (stacked_bar/grouped_bar/line/area), updated plot branches and environment-aware save/show, examples updated to new API usages and logging enabling.
Configuration & init
flixopt/config.py, flixopt/__init__.py
Logging defaults changed (no file by default), added show_logger_name and console stream options, formatter changes; version fallback and warning filters added.
Results, persistence & tests
flixopt/results.py, tests/**
CalculationResults now stores flow_system_data and reconstructs FlowSystem on demand; many tests converted to submodel/coords-aware fixtures; new scenario and conversion-path tests; results filtering/indexing expanded.
Documentation & tooling
docs/**, mkdocs.yml, scripts/gen_ref_pages.py, README.md
Rewrites/adds mathematical-notation and modeling-pattern docs, reorganizes navigation, removes some old pages, updates README and doc generation script.
Packaging & CI
pyproject.toml, MANIFEST.in, .github/workflows/python-app.yaml
Conditional numexpr dep, tightened MANIFEST.in, pytest warning rules and Bandit config, expanded workflow branch triggers and adjusted pytest args.
Changelog & examples
CHANGELOG.md, examples/*
New Unreleased and [3.0.0] entries documenting breaking changes and API renames; adds scenario and two-stage optimization examples; examples updated to inputs/outputs/share_from_* usages.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant FS as FlowSystem
  participant FM as FlowSystemModel
  participant SM as Submodels
  participant Calc as Calculation
  participant Solver as Solver
  participant Res as CalculationResults

  User->>FS: define elements, timesteps, periods, scenarios
  FS->>FS: connect_and_transform()
  User->>Calc: FullCalculation(FS)
  Calc->>FS: create_model(normalize_weights)
  FS-->>FM: FlowSystemModel created
  FM->>SM: add_submodels(Flows/Components/Effects)
  Calc->>FM: do_modeling() -> Calculation (returned)
  Calc->>Solver: solve()
  Solver-->>Calc: solution
  Calc->>Res: CalculationResults.from_calculation(Calc)
  Res->>FS: flow_system_data = FS.to_dataset()
  Res-->>User: results (flows/components/effects)
Loading
sequenceDiagram
  autonumber
  actor User
  participant Res as CalculationResults
  participant DS as xr.Dataset
  participant FS as FlowSystem

  User->>Res: from_file(folder)
  Res->>DS: load solution + flow_system_data
  User->>Res: access .flow_system
  Res->>FS: FlowSystem.from_dataset(DS)
  FS-->>Res: FlowSystem (reconstructed)
  Res-->>User: FlowSystem object
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120+ minutes

Possibly related PRs

Suggested reviewers

  • baumbude
  • PStange

Poem

A rabbit taps the keys—thump, thump—
Submodels sprout where old models used to clump.
Time, period, scenarios hop in a row,
Effects share carrots where conversions flow.
Data burrows deep — results gleam bright as snow. 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 45.56% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title concisely highlights the major addition of multi-period and stochastic modeling support for the v3.0.0 release, directly reflecting the primary changes in the pull request.
Description Check ✅ Passed The pull request description follows the repository’s template by including all required sections, clearly identifying the new feature type and checklist items, though the Related Issues section still contains a placeholder and the Testing boxes remain unchecked.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8cb3866 and c8c1417.

📒 Files selected for processing (7)
  • .github/workflows/python-app.yaml (2 hunks)
  • CHANGELOG.md (3 hunks)
  • docs/index.md (1 hunks)
  • docs/user-guide/recipes/index.md (1 hunks)
  • examples/02_Complex/complex_example.py (7 hunks)
  • flixopt/__init__.py (2 hunks)
  • pyproject.toml (2 hunks)

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 16

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
tests/test_storage.py (1)

481-487: Fix boolean comparison on xarray variables.

var.upper == 1 and var.lower == 1 is ambiguous for arrays and will error. Use array-wise checks.

Apply this diff:

-            if 'InvestStorage|is_invested' in model.variables:
-                var = model.variables['InvestStorage|is_invested']
-                # Check if the lower and upper bounds are both 1
-                assert var.upper == 1 and var.lower == 1, (
-                    'is_invested variable should be fixed to 1 when optional=False'
-                )
+            if 'InvestStorage|is_invested' in model.variables:
+                var = model.variables['InvestStorage|is_invested']
+                import numpy as np
+                assert np.allclose(var.upper, 1) and np.allclose(var.lower, 1), \
+                    'is_invested variable should be fixed to 1 when optional=False'
flixopt/aggregation.py (1)

288-305: Modeling hook not executed: rename do_modeling to _do_modeling.

Submodel calls _do_modeling() in its constructor; your override is named do_modeling, so nothing runs and no constraints/variables are created.

Apply this diff:

-class AggregationModel(Submodel):
+class AggregationModel(Submodel):
@@
-    def do_modeling(self):
+    def _do_modeling(self):

Also applies to: 310-316

flixopt/io.py (1)

198-206: Confirm attr roundtrip compatibility with existing artifacts.

load_dataset_from_netcdf already supports both formats: it json.loads when a top-level 'attrs' key exists and leaves plain attrs untouched; save_dataset_to_netcdf writes all attrs as JSON under the 'attrs' key—so the two functions are symmetric for JSON-serializable attrs (flixopt/io.py: save_dataset_to_netcdf, load_dataset_from_netcdf).

  • Risk: save_dataset_to_netcdf uses json.dumps on attrs — this will fail or lose fidelity for non‑JSON‑serializable values (numpy dtypes/arrays, pandas Timestamps/Indices, bytes, sets, custom objects). This is a breaking change for datasets with such attrs.
  • Action: convert/normalize attrs before json.dumps (numpy→Python scalars, arrays→lists, datetimes→ISO strings) or provide a robust serializer (json.dumps(..., default=...), or a safe fallback like pickle) and add unit tests that save+load datasets including nested attrs with numpy/pandas types and an older-format file load test.

Location: flixopt/io.py — save_dataset_to_netcdf / load_dataset_from_netcdf.

flixopt/linear_converters.py (1)

608-614: Fix: check_bounds fails on xr.DataArray (ambiguous truth).

When value/lower/upper are xr.DataArray (not TimeSeriesData), np.all(...) returns a DataArray and the if condition raises "truth value of a DataArray is ambiguous". Convert xr.DataArray to numpy as done for TimeSeriesData.

Apply this diff and add the missing import:

@@
-    if isinstance(value, TimeSeriesData):
+    if isinstance(value, TimeSeriesData):
         value = value.data
-    if isinstance(lower_bound, TimeSeriesData):
+    elif isinstance(value, xr.DataArray):
+        value = value.data
+    if isinstance(lower_bound, TimeSeriesData):
         lower_bound = lower_bound.data
-    if isinstance(upper_bound, TimeSeriesData):
+    elif isinstance(lower_bound, xr.DataArray):
+        lower_bound = lower_bound.data
+    if isinstance(upper_bound, TimeSeriesData):
         upper_bound = upper_bound.data
+    elif isinstance(upper_bound, xr.DataArray):
+        upper_bound = upper_bound.data

Outside this hunk, add:

+import xarray as xr
🧹 Nitpick comments (51)
CHANGELOG.md (7)

36-39: Fix heading level and tighten wording for multi-period section

Use h3 (###), not h4, and simplify phrasing.

-#### Multi-period-support
-A flixopt model might be modeled with a "year" dimension.
-This enables to model transformation pathways over multiple years.
+### Multi-period support
+A flixopt model can be modeled with a 'year' dimension.
+This enables modeling transformation pathways over multiple years.

40-46: Fix heading level and phrasing in stochastic section

Normalize heading and wording.

-#### Stochastic modeling
-A flixopt model can be modeled with a scenario dimension.
-Scenarios can be weighted and variables can be equated across scenarios. This enables to model uncertainties in the flow system, such as:
+### Stochastic modeling
+A flixopt model can include a 'scenario' dimension.
+Scenarios can be weighted and variables can be linked across scenarios. This enables modeling uncertainties in the flow system, such as:

53-66: Fix heading level and list indentation (markdownlint MD001/MD007)

Promote to h3 and use consistent 2-space indentation for nested bullets.

-#### Improved Data handling: IO, resampling and more through xarray
-* IO for all Interfaces and the FlowSystem with round-trip serialization support
-    * NetCDF export/import capabilities for all Interface objects and FlowSystem
-    * JSON export for documentation purposes
-    * Recursive handling of nested Interface objects
-* FlowSystem data manipulation methods
-   * `sel()` and `isel()` methods for temporal data selection
-   * `resample()` method for temporal resampling
-   * `copy()` method to create a copy of a FlowSystem, including all underlying Elements and their data
-   * `__eq__()` method for FlowSystem comparison
-
-* Core data handling improvements
-   * `get_dataarray_stats()` function for statistical summaries
-   * Enhanced `DataConverter` class with better TimeSeriesData support
+### Improved data handling: I/O, resampling, and more through xarray
+* I/O for all Interfaces and the FlowSystem with round-trip serialization support
+  * NetCDF export/import capabilities for all Interface objects and FlowSystem
+  * JSON export for documentation purposes
+  * Recursive handling of nested Interface objects
+* FlowSystem data manipulation methods
+  * `sel()` and `isel()` methods for temporal data selection
+  * `resample()` method for temporal resampling
+  * `copy()` method to create a copy of a FlowSystem, including all underlying Elements and their data
+  * `__eq__()` method for FlowSystem comparison
+* Core data handling improvements
+  * `get_dataarray_stats()` function for statistical summaries
+  * Enhanced `DataConverter` class with better TimeSeriesData support

69-78: Correct typos and naming in “Added” bullets

Fix spelling, casing, and clarity.

-### Added
-* FlowSystem Restoring: The used FlowSystem will now get restired from the results (lazily). ALll Parameters can be safely acessed anytime after the solve.
-* FLowResults added as a new class to store the results of Flows. They can now be accessed directly.
-* Added precomputed DataArrays for `size`s, `flow_rate`s and `flow_hour`s.
+### Added
+* FlowSystem restoring: The used FlowSystem will now get restored from the results (lazily). All parameters can be safely accessed anytime after the solve.
+* FlowResults added as a new class to store the results of flows. They can now be accessed directly.
+* Added precomputed DataArrays for `sizes`, `flow_rates`, and `flow_hours`.
 * Added `effects_per_component()`-Dataset to Results that stores the direct (and indirect) effects of each component. This greatly improves the evaluation of the impact of individual Components, even with many and complex effects.
 * Improved filter methods for Results
-* Balanced storage - Storage charging and discharging sizes can now be forced to be equal in when optimizing their size.
-* Added Example for 2-stage Investment decisions leveraging the resampling of a FlowSystem
-* New Storage Parameter: `relative_minimum_final_charge_state` and `relative_maximum_final_charge_state` parameter for final state control
+* Balanced storage – storage charging and discharging sizes can now be forced to be equal when optimizing their size.
+* Added example for 2‑stage investment decisions leveraging resampling of a FlowSystem
+* New storage parameters: `relative_minimum_final_charge_state` and `relative_maximum_final_charge_state` for final state control

89-91: Minor typo in “Changed” bullets

Fix “acess” -> “access”.

-  * Submodel: The base class for all submodels. Each is a subset of the Model, for simpler acess and clearer code.
+  * Submodel: The base class for all submodels. Each is a subset of the Model, for simpler access and clearer code.

105-107: Polish “Known issues” text and fix typos

Clarify and correct spelling.

-* IO for single Interfaces/Elemenets to Datasets might not work properly if the Interface/Element is not part of a fully transformed and connected FlowSystem. This arrises from Numeric Data not being stored as xr.DataArray by the user. To avoid this, always use the `to_dataset()` on Elements inside a FlowSystem thats connected and transformed.
+* I/O for single Interfaces/Elements to Datasets might not work properly if the Interface/Element is not part of a fully transformed and connected FlowSystem. This arises from numeric data not being stored as `xr.DataArray` by the user. To avoid this, always use `to_dataset()` on Elements inside a FlowSystem that’s connected and transformed.

108-121: Deduplicate items and fix markdown emphasis (MD050) in “Development”

Remove repeated renames; fix spacing and wrap repr in backticks; normalize list indent.

 ### *Development*
 * **BREAKING**: Calculation.do_modeling() now returns the Calculation object instead of its linopy.Model
-* **BREAKING**: Renamed class `SystemModel` to `FlowSystemModel`
-* **BREAKING**: Renamed class `Model` to `Submodel`
 * FlowSystem data management simplified - removed `time_series_collection` pattern in favor of direct timestep properties
 * Change modeling hierarchy to allow for more flexibility in future development. This leads to minimal changes in the access and creation of Submodels and their variables.
-* Added new module `.modeling`that contains Modelling primitives and utilities
+* Added new module `.modeling` that contains modeling primitives and utilities
 * Clearer separation between the main Model and "Submodels"
 * Improved access to the Submodels and their variables, constraints and submodels
-* Added __repr__() for Submodels to easily inspect its content
+* Added `__repr__()` for Submodels to easily inspect its content
 * Enhanced data handling methods
-   * `fit_to_model_coords()` method for data alignment
-   * `fit_effects_to_model_coords()` method for effect data processing
-   * `connect_and_transform()` method replacing several operations
+  * `fit_to_model_coords()` method for data alignment
+  * `fit_effects_to_model_coords()` method for effect data processing
+  * `connect_and_transform()` method replacing several operations
examples/05_Two-stage-optimization/two_stage_optimization.py (2)

2-8: Fix typos in module docstring

Minor grammar/typos.

-This can be very useful when working with large models or during developement state,
+This can be very useful when working with large models or during development,
@@
-A common use case is to do optimize the investments of a model with a downsampled version of the original model, and than fix the computed sizes when calculating th actual dispatch.
+A common use case is to optimize the investments of a model with a downsampled version of the original model, and then fix the computed sizes when calculating the actual dispatch.

127-131: xarray boolean comparison needs scalar conversion; fix log text

Use .all().item() for a Python bool; fix “where”→“were”.

-if (calculation_dispatch.results.sizes().round(5) == calculation_sizing.results.sizes().round(5)).all():
-        logger.info('Sizes where correctly equalized')
+if (calculation_dispatch.results.sizes().round(5) == calculation_sizing.results.sizes().round(5)).all().item():
+        logger.info('Sizes were correctly equalized')
     else:
-        raise RuntimeError('Sizes where not correctly equalized')
+        raise RuntimeError('Sizes were not correctly equalized')
examples/03_Calculation_types/example_calculation_types.py (1)

136-141: Use effect label key instead of Effect object in mapping (deprecated)

Align with deprecation notice; use CO2.label.

-        source=fx.Flow(
-            'P_el', bus='Strom', size=1000, effects_per_flow_hour={costs.label: TS_electricity_price_buy, CO2: 0.3}
-        ),
+        source=fx.Flow(
+            'P_el', bus='Strom', size=1000, effects_per_flow_hour={costs.label: TS_electricity_price_buy, CO2.label: 0.3}
+        ),
tests/run_all_tests.py (1)

10-10: Confirm intention to only run test_integration

If not intentional, run full suite by default.

-    pytest.main(['test_integration.py', '--disable-warnings'])
+    pytest.main(['-q', '--disable-warnings'])
tests/test_functional.py (1)

217-249: Verify the capitalization change in error messages.

The error messages have been changed from 'costs doesnt match expected value' to 'costs doesnt match expected value' with inconsistent capitalization. While this is minor, it would be better to maintain consistency.

Consider standardizing the error messages:

-            'costs doesnt match expected value'
+            'costs does not match expected value'
tests/test_integration.py (1)

309-316: Fix inconsistent error message formatting.

The error messages use different formats: lowercase "costs do not match" vs other patterns. Consider standardizing.

-                f'costs do not match for {modeling_type} modeling type',
+                f'Costs do not match for {modeling_type} modeling type',
tests/conftest.py (1)

44-63: Consider adding docstring to the coords_config fixture.

This fixture provides critical test configurations for different coordinate combinations. Adding a docstring would improve maintainability.

 @pytest.fixture(
     params=[
         {'timesteps': pd.date_range('2020-01-01', periods=10, freq='h', name='time'), 'years': None, 'scenarios': None},
         {
             'timesteps': pd.date_range('2020-01-01', periods=10, freq='h', name='time'),
             'years': pd.Index([2020, 2030, 2040], name='year'),
             'scenarios': None,
         },
         {
             'timesteps': pd.date_range('2020-01-01', periods=10, freq='h', name='time'),
             'years': pd.Index([2020, 2030, 2040], name='year'),
             'scenarios': pd.Index(['A', 'B'], name='scenario'),
         },
     ],
     ids=['time_only', 'time+years', 'time+years+scenarios'],
 )
 def coords_config(request):
-    """Coordinate configurations for parametrized testing."""
+    """
+    Coordinate configurations for parametrized testing.
+    
+    Returns:
+        dict: Configuration with 'timesteps', 'years', and 'scenarios' keys.
+              Tests will run with each parameter combination.
+    """
     return request.param
tests/test_effects_shares_summation.py (1)

95-126: Fix incorrect comment on line 124.

The comment shows the wrong total value. The comment should reflect the correct calculation.

     # Effect1 -> Effect3 has two paths:
     # 1. Effect1 -> Effect2 -> Effect3 = 1.1 * 5.0 = 5.5
     # 2. Effect1 -> Effect3 = 1.2
-    # Total = 0.6 + 2.75 = 3.35
+    # Total = 5.5 + 1.2 = 6.7
     assert result[('Effect1', 'Effect3')].item() == 1.2 + 1.1 * 5.0
examples/04_Scenarios/scenario_example.py (3)

1-9: Consider adding more comprehensive module docstring.

The docstring could be expanded to better describe what the example demonstrates.

 """
-This script shows how to use the flixopt framework to model a simple energy system.
+This script demonstrates how to use the flixopt framework to model a simple energy system
+with multiple scenarios, years, and various components including:
+- Energy buses (electricity, heat, gas)
+- Conversion components (Boiler, CHP)
+- Storage systems
+- Sources and sinks
+- Multi-period optimization with scenarios
 """

122-129: Remove duplicate plot_heatmap call.

Line 122 and line 128 contain the same plot call for 'CHP(Q_th)|flow_rate'.

     calculation.results.plot_heatmap('CHP(Q_th)|flow_rate')
 
     # --- Analyze Results ---
     calculation.results['Fernwärme'].plot_node_balance_pie()
     calculation.results['Fernwärme'].plot_node_balance(style='stacked_bar')
     calculation.results['Storage'].plot_node_balance()
-    calculation.results.plot_heatmap('CHP(Q_th)|flow_rate')

134-138: Consider handling plot return values consistently.

Line 137 assigns the plot result to fig, ax but doesn't use them. Either use the variables or remove the assignment for consistency.

     # Save results to file for later usage
     calculation.results.to_file()
-    fig, ax = calculation.results['Storage'].plot_charge_state(engine='matplotlib')
+    calculation.results['Storage'].plot_charge_state(engine='matplotlib')
flixopt/plotting.py (4)

201-205: Broaden exception handling for invalid colormap names.

px.colors.get_colorscale typically raises ValueError for unknown scales, not PlotlyError. Catch both to ensure fallback works.

Apply this diff:

-            try:
-                colorscale = px.colors.get_colorscale(colormap_name)
-            except PlotlyError as e:
+            try:
+                colorscale = px.colors.get_colorscale(colormap_name)
+            except (PlotlyError, ValueError) as e:
                 logger.warning(f"Colorscale '{colormap_name}' not found in Plotly. Using {self.default_colormap}: {e}")
                 colorscale = px.colors.get_colorscale(self.default_colormap)

Also applies to: 213-216


330-336: Add a deprecated mode alias for backward compatibility.

Public API changed from mode to style. Keep a soft-landing path to avoid breaking external users.

Apply this diff in the signature and early in the function:

 def with_plotly(
     data: pd.DataFrame,
-    style: Literal['stacked_bar', 'line', 'area', 'grouped_bar'] = 'stacked_bar',
+    style: Literal['stacked_bar', 'line', 'area', 'grouped_bar'] = 'stacked_bar',
+    mode: Literal['bar', 'line', 'area'] | None = None,
     colors: ColorType = 'viridis',
     title: str = '',
     ylabel: str = '',
     xlabel: str = 'Time in h',
     fig: go.Figure | None = None,
 ) -> go.Figure:
@@
-    if style not in ('stacked_bar', 'line', 'area', 'grouped_bar'):
+    if mode is not None:
+        import warnings
+        warnings.warn("Parameter 'mode' is deprecated; use 'style' instead.", DeprecationWarning, stacklevel=2)
+        style = {'bar': 'stacked_bar', 'line': 'line', 'area': 'area'}.get(mode, style)
+    if style not in ('stacked_bar', 'line', 'area', 'grouped_bar'):
         raise ValueError(f"'style' must be one of {{'stacked_bar','line','area', 'grouped_bar'}}, got {style!r}")

Also applies to: 357-358


366-393: Use elif for mutually exclusive styles and fix misleading comments.

  • Switch second if to elif to avoid unnecessary checks.
  • Comments contradict values: bargap=0.2 is not “No space”.

Apply this diff:

-    if style == 'grouped_bar':
+    elif style == 'grouped_bar':
@@
-        fig.update_layout(
-            barmode='group',
-            bargap=0.2,  # No space between bars
-            bargroupgap=0,  # space between grouped bars
-        )
+        fig.update_layout(
+            barmode='group',
+            bargap=0.2,       # Small space between bars
+            bargroupgap=0,    # No space within grouped bars
+        )

470-507: Potential width unit issue for datetime x-axis in Matplotlib bars.

width = data.index.to_series().diff().dropna().min() yields a Timedelta for datetime indexes; Matplotlib bar expects numeric in axis units. Convert Timedelta to days (or use a reasonable default) to avoid backend-dependent errors.

Suggested change:

-        width = data.index.to_series().diff().dropna().min()  # Minimum time difference
+        delta = data.index.to_series().diff().dropna().min()
+        if isinstance(delta, pd.Timedelta):
+            width = delta / pd.Timedelta(days=1)  # Matplotlib date units are days
+        else:
+            width = delta if delta else 0.8

Also applies to: 514-518

tests/test_scenarios.py (2)

14-112: Unused fixture test_system.

This fixture isn’t referenced by tests. Remove or wire it into a test to avoid dead code and reduce CI time.


282-287: Redundant re-solve after IO roundtrip.

The second create_calculation_and_solve call re-runs the same model immediately after loading, without validating against persisted results. Drop it or assert equivalence to reduce runtime.

Apply this diff:

-    calc = create_calculation_and_solve(
-        flow_system_piecewise_conversion_scenarios,
-        solver=fx.solvers.GurobiSolver(mip_gap=0.01, time_limit_seconds=60),
-        name='test_full_scenario',
-    )
+    # Optionally compare against persisted results here if needed.
flixopt/io.py (2)

66-66: Limit YAML representer to the custom dumper to avoid global side effects.

Registering the representer on yaml.SafeDumper is global. Attach it to CustomDumper to prevent affecting other dumps.

Apply this diff:

-    yaml.add_representer(str, represent_str, Dumper=yaml.SafeDumper)
+    class CustomDumper(yaml.SafeDumper):
+        def increase_indent(self, flow=False, indentless=False):
+            return super().increase_indent(flow, False)
+    CustomDumper.add_representer(str, represent_str)

Also applies to: 69-72


229-235: Make attribute JSON serialization robust.

json.dumps will fail on non-JSONable attrs (e.g., numpy types). Provide a fallback via default=str.

Apply this diff:

-    ds.attrs = {'attrs': json.dumps(ds.attrs)}
+    ds.attrs = {'attrs': json.dumps(ds.attrs, default=str)}
@@
-        if data_var.attrs:  # Only if there are attrs
-            ds[var_name].attrs = {'attrs': json.dumps(data_var.attrs)}
+        if data_var.attrs:  # Only if there are attrs
+            ds[var_name].attrs = {'attrs': json.dumps(data_var.attrs, default=str)}
@@
-        if hasattr(coord_var, 'attrs') and coord_var.attrs:
-            ds[coord_name].attrs = {'attrs': json.dumps(coord_var.attrs)}
+        if hasattr(coord_var, 'attrs') and coord_var.attrs:
+            ds[coord_name].attrs = {'attrs': json.dumps(coord_var.attrs, default=str)}

Also applies to: 237-240

tests/test_storage.py (2)

14-15: Simplify fixture unpacking for clarity.

flow_system, coords_config = basic_flow_system_linopy_coords, coords_config is redundant. Assign directly.

Example:

-        flow_system, coords_config = basic_flow_system_linopy_coords, coords_config
+        flow_system = basic_flow_system_linopy_coords

Apply similarly to other tests.

Also applies to: 85-88, 170-172, 256-257, 308-309, 353-354, 385-387, 442-443


53-62: Avoid mutating the model by creating comparison variables.

Using model.add_variables(...) inside tests mutates the model under test. Prefer constructing a separate (temporary) model or building expected linopy.Variable objects without registering them.

Also applies to: 128-138, 211-231

flixopt/aggregation.py (2)

326-334: Indexing linopy.Variables with a set can be unstable.

Sets are unordered. Convert to a sorted list to ensure determinism.

Apply this diff:

-            all_variables_of_component = set(component.submodel.variables)
+            all_variables_of_component = set(component.submodel.variables)
@@
-                relevant_variables = component.submodel.variables[all_variables_of_component & time_variables]
+                relevant_variables = component.submodel.variables[sorted(all_variables_of_component & time_variables)]
             else:
-                relevant_variables = component.submodel.variables[all_variables_of_component & binary_time_variables]
+                relevant_variables = component.submodel.variables[sorted(all_variables_of_component & binary_time_variables)]

144-169: Update plotting calls to new style keyword for readability.

Positional 'line' works but is brittle. Prefer explicit style='line' for both calls.

Apply this diff:

-        fig = plotting.with_plotly(df_org, 'line', colors=colormap)
+        fig = plotting.with_plotly(df_org, style='line', colors=colormap)
@@
-        fig = plotting.with_plotly(df_agg, 'line', colors=colormap, fig=fig)
+        fig = plotting.with_plotly(df_agg, style='line', colors=colormap, fig=fig)

Also applies to: 153-157

flixopt/linear_converters.py (2)

76-96: Initialize eta via setter to enforce bounds on creation.

Currently eta bounds aren't checked at construction. Set via the property to run check_bounds.

         self.Q_fu = Q_fu
         self.Q_th = Q_th
+        # ensure bounds check on initialization
+        self.eta = eta

163-171: Initialize eta via setter to enforce bounds on creation.

Mirror Boiler: call the setter so eta is validated at init.

         self.P_el = P_el
         self.Q_th = Q_th
+        # ensure bounds check on initialization
+        self.eta = eta

Also applies to: 181-192

tests/test_component.py (1)

31-34: Remove redundant fixture re-assignment.

The tuple assignment rebinds variables to themselves and is a no-op. Drop it to reduce noise.

Example (apply similarly to all occurrences):

-        flow_system, coords_config = basic_flow_system_linopy_coords, coords_config
+        flow_system = basic_flow_system_linopy_coords

Also applies to: 72-76, 175-178, 235-237, 370-372

flixopt/core.py (1)

315-321: Optional: accept 1D DataArray with matching coordinates but different dim name.

Current _broadcast_to_target requires source dim names to match target dims exactly. Many users construct xr.DataArray(v, coords=(time_index,)) which creates dim_0. Consider auto-mapping a single source dim to a target dim if coordinates match by values.

If you want to pursue this, I can propose a minimal patch that attempts dim renaming when len(data.dims)==1 and data.coords[dim].equals(coords[target_dim]).

tests/test_linear_converter.py (2)

53-54: Name the time dimension explicitly to reduce fragility.

Construct the DataArray with dim 'time' so downstream conversion/broadcasting doesn't rely on dim-name relaxation.

-        efficiency_series = xr.DataArray(varying_efficiency, coords=(timesteps,))
+        efficiency_series = xr.DataArray(varying_efficiency, coords={'time': timesteps}, dims='time')

248-256: Same here: prefer explicit dim when building fluctuating_cop.

Keeps consistency with the rest of the suite.

-        conversion_factors = [{input_flow.label: fluctuating_cop, output_flow.label: np.ones(len(timesteps))}]
+        conversion_factors = [
+            {input_flow.label: xr.DataArray(fluctuating_cop, coords={'time': timesteps}, dims='time'),
+             output_flow.label: xr.DataArray(np.ones(len(timesteps)), coords={'time': timesteps}, dims='time')}
+        ]
flixopt/elements.py (1)

488-490: Nit: typo in error message.

"notsupported" -> "not yet supported".

-                    f'Different values in different years or scenarios are not yetsupported.'
+                    f'Different values in different years or scenarios are not yet supported.'
flixopt/modeling.py (1)

522-524: Fix error message: wrong function name

Copy-paste from another method.

Apply:

-        if not isinstance(model, Submodel):
-            raise ValueError('BoundingPatterns.active_bounds_with_state() can only be used with a Submodel')
+        if not isinstance(model, Submodel):
+            raise ValueError('BoundingPatterns.scaled_bounds_with_state() can only be used with a Submodel')
flixopt/interface.py (2)

921-924: Fix typos in warning text

Minor text quality.

Apply:

-                        'When using investment_scenarios, minimum_size and fixed_size should only ne used if optional is True.'
-                        'Otherwise the investment cannot be 0 incertain scenarios while being non-zero in others.'
+                        'When using investment_scenarios, minimum_size and fixed_size should only be used if optional is True. '
+                        'Otherwise, the investment cannot be 0 in certain scenarios while being non-zero in others.'

1186-1189: Nit: spelling in docstring

“wether” → “whether”.

Apply:

-        """Determines wether a Variable for SWITCH-ON is needed or not"""
+        """Determines whether a variable for SWITCH-ON is needed or not"""
flixopt/calculation.py (3)

160-168: Correct deprecation message in active_timesteps property

Message self-references; point to FlowSystem.sel/isel.

Apply:

-        warnings.warn(
-            'active_timesteps is deprecated. Use active_timesteps instead.',
+        warnings.warn(
+            "active_timesteps is deprecated. Use flow_system.sel(time=...) or flow_system.isel(time=...) instead.",
             DeprecationWarning,
             stacklevel=2,
         )

326-338: Prefer robust scalar extraction for timestep sizes in aggregation

Use xarray reductions and cast to float to avoid ndarray/DataArray arithmetic pitfalls.

Apply:

-        dt_min, dt_max = (
-            np.min(self.flow_system.hours_per_timestep),
-            np.max(self.flow_system.hours_per_timestep),
-        )
+        dt_min = float(self.flow_system.hours_per_timestep.min().item())
+        dt_max = float(self.flow_system.hours_per_timestep.max().item())
@@
-        steps_per_period = self.aggregation_parameters.hours_per_period / self.flow_system.hours_per_timestep.max()
-        is_integer = (
-            self.aggregation_parameters.hours_per_period % self.flow_system.hours_per_timestep.max()
-        ).item() == 0
-        if not (steps_per_period.size == 1 and is_integer):
+        steps_per_period = self.aggregation_parameters.hours_per_period / dt_max
+        is_integer = (self.aggregation_parameters.hours_per_period % dt_max) == 0
+        if not is_integer:
             raise ValueError(
                 f'The selected {self.aggregation_parameters.hours_per_period=} does not match the time '
                 f'step size of {dt_min} hours). It must be a multiple of {dt_min} hours.'
             )

145-146: Use already-rounded main_results

You call round_nested_floats in main_results; duplicate rounding here is redundant.

flixopt/components.py (1)

860-865: Balanced sizes: remove no-op multiplications

* 1 is redundant and can be dropped.

flixopt/effects.py (4)

499-501: Clarify error message for per-hour bounds without time-dim

The condition triggers if either bound is provided, not “both”. Update message to avoid confusion.

-            raise ValueError('Both max_per_hour and min_per_hour cannot be used when has_time_dim is False')
+            raise ValueError("max_per_hour and/or min_per_hour cannot be used when 'time' is not in dims")

562-597: Avoid O(n²) queue by using deque in BFS

Using list.pop(0) makes the traversal quadratic. Switch to deque for linear-time iteration.

+from collections import deque
@@
-        queue = [(origin, 1, [origin])]
+        queue = deque([(origin, 1, [origin])])
@@
-        while queue:
-            current_domain, factor, path = queue.pop(0)
+        while queue:
+            current_domain, factor, path = queue.popleft()
@@
-                queue.append((target, indirect_factor, new_path))
+                queue.append((target, indirect_factor, new_path))

337-357: Guard scalar-effect input when no standard effect is set

If a scalar is provided but no standard effect exists, this will raise in the property access. Fail fast with a clear error.

         if effect_values_user is None:
             return None
-        if isinstance(effect_values_user, dict):
+        if isinstance(effect_values_user, dict):
             return {get_effect_label(effect): value for effect, value in effect_values_user.items()}
-        return {self.standard_effect.label: effect_values_user}
+        if self._standard_effect is None:
+            raise KeyError(
+                'Scalar effect value provided but no standard effect is configured. '
+                'Either set an effect as is_standard=True or provide a mapping {effect_label: value}.'
+            )
+        return {self.standard_effect.label: effect_values_user}

12-12: Remove duplicate Iterator import

Iterator is imported at runtime and under TYPE_CHECKING. Keep type-only import to reduce runtime surface.

-from collections.abc import Iterator
flixopt/features.py (1)

169-179: Pass coordinates, not dims list, to tracking variable

ModelingPrimitives likely expects coords (Coordinates), not a raw list. Use model.get_coords for ['year','scenario'].

-            short_name='on_hours_total',
-            coords=['year', 'scenario'],
+            short_name='on_hours_total',
+            coords=self._model.get_coords(['year', 'scenario']),
flixopt/flow_system.py (1)

467-474: Align attribute naming: use .model consistently

FlowSystem.init defines self.model but create_model assigns self.submodel. Prefer one attribute to avoid confusion and downstream bugs.

-        self.submodel = FlowSystemModel(self)
-        return self.submodel
+        self.model = FlowSystemModel(self)
+        return self.model
flixopt/results.py (1)

295-301: Avoid shadowing FlowSystem import

Redundant re-import risks shadowing; use the module-level import already present.

-                from . import FlowSystem
-
                 current_logger_level = logger.getEffectiveLevel()
                 logger.setLevel(logging.CRITICAL)
                 self._flow_system = FlowSystem.from_dataset(self.flow_system_data)
                 self._flow_system._connect_network()
                 logger.setLevel(current_logger_level)
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bb17bd4 and af4a297.

⛔ Files ignored due to path filters (3)
  • docs/images/flixopt-icon.svg is excluded by !**/*.svg
  • examples/05_Two-stage-optimization/Zeitreihen2020.csv is excluded by !**/*.csv
  • pics/flixopt-icon.svg is excluded by !**/*.svg
📒 Files selected for processing (40)
  • CHANGELOG.md (1 hunks)
  • examples/01_Simple/simple_example.py (1 hunks)
  • examples/03_Calculation_types/example_calculation_types.py (4 hunks)
  • examples/04_Scenarios/scenario_example.py (1 hunks)
  • examples/05_Two-stage-optimization/two_stage_optimization.py (1 hunks)
  • flixopt/aggregation.py (6 hunks)
  • flixopt/calculation.py (14 hunks)
  • flixopt/components.py (12 hunks)
  • flixopt/core.py (3 hunks)
  • flixopt/effects.py (10 hunks)
  • flixopt/elements.py (9 hunks)
  • flixopt/features.py (7 hunks)
  • flixopt/flow_system.py (10 hunks)
  • flixopt/interface.py (8 hunks)
  • flixopt/io.py (5 hunks)
  • flixopt/linear_converters.py (10 hunks)
  • flixopt/modeling.py (1 hunks)
  • flixopt/plotting.py (8 hunks)
  • flixopt/results.py (30 hunks)
  • flixopt/structure.py (6 hunks)
  • flixopt/utils.py (1 hunks)
  • tests/conftest.py (11 hunks)
  • tests/run_all_tests.py (1 hunks)
  • tests/test_bus.py (4 hunks)
  • tests/test_component.py (5 hunks)
  • tests/test_cycle_detection.py (1 hunks)
  • tests/test_dataconverter.py (1 hunks)
  • tests/test_effect.py (6 hunks)
  • tests/test_effects_shares_summation.py (1 hunks)
  • tests/test_flow.py (16 hunks)
  • tests/test_functional.py (11 hunks)
  • tests/test_integration.py (4 hunks)
  • tests/test_io.py (4 hunks)
  • tests/test_linear_converter.py (14 hunks)
  • tests/test_on_hours_computation.py (1 hunks)
  • tests/test_plots.py (1 hunks)
  • tests/test_results_plots.py (1 hunks)
  • tests/test_scenarios.py (1 hunks)
  • tests/test_storage.py (10 hunks)
  • tests/test_timeseries.py (0 hunks)
💤 Files with no reviewable changes (1)
  • tests/test_timeseries.py
🧰 Additional context used
🧬 Code graph analysis (34)
tests/test_plots.py (1)
flixopt/plotting.py (2)
  • with_plotly (328-466)
  • with_matplotlib (469-562)
tests/test_results_plots.py (1)
flixopt/results.py (1)
  • plot_charge_state (1134-1206)
examples/05_Two-stage-optimization/two_stage_optimization.py (3)
flixopt/results.py (3)
  • flow_system (290-307)
  • size (1283-1291)
  • sizes (434-461)
flixopt/flow_system.py (3)
  • FlowSystem (46-879)
  • resample (825-875)
  • sel (755-788)
flixopt/calculation.py (5)
  • FullCalculation (174-258)
  • do_modeling (182-190)
  • do_modeling (303-317)
  • solve (219-258)
  • fix_sizes (192-217)
tests/test_effects_shares_summation.py (1)
flixopt/effects.py (2)
  • effects (411-412)
  • calculate_all_conversion_paths (533-601)
tests/test_cycle_detection.py (1)
flixopt/effects.py (2)
  • effects (411-412)
  • detect_cycles (604-651)
tests/test_integration.py (3)
flixopt/structure.py (2)
  • solution (106-129)
  • values (991-992)
tests/conftest.py (1)
  • assert_almost_equal_numeric (669-681)
flixopt/results.py (6)
  • solution_without_overlap (1458-1471)
  • to_file (776-823)
  • to_file (1509-1532)
  • SegmentedCalculationResults (1294-1532)
  • from_file (119-150)
  • from_file (1402-1426)
tests/test_io.py (3)
tests/conftest.py (2)
  • simple_flow_system_scenarios (412-440)
  • simple_flow_system (382-408)
flixopt/flow_system.py (4)
  • plot_network (475-512)
  • to_json (330-344)
  • to_dataset (229-241)
  • FlowSystem (46-879)
flixopt/structure.py (2)
  • to_json (631-648)
  • to_dataset (494-517)
flixopt/io.py (2)
flixopt/structure.py (2)
  • items (985-986)
  • copy (687-699)
flixopt/flow_system.py (1)
  • coords (743-749)
tests/test_bus.py (4)
tests/conftest.py (5)
  • basic_flow_system_linopy_coords (645-660)
  • coords_config (60-62)
  • assert_conequal (705-721)
  • assert_var_equal (724-758)
  • create_linopy_model (699-702)
flixopt/structure.py (4)
  • variables (894-900)
  • constraints (885-891)
  • get_coords (139-168)
  • get_coords (840-845)
flixopt/flow_system.py (2)
  • coords (743-749)
  • add_elements (441-465)
flixopt/elements.py (2)
  • Bus (119-207)
  • Flow (224-504)
flixopt/linear_converters.py (1)
flixopt/core.py (1)
  • TimeSeriesData (41-149)
examples/03_Calculation_types/example_calculation_types.py (3)
flixopt/core.py (3)
  • TimeSeriesData (41-149)
  • aggregation_weight (108-109)
  • aggregation_group (104-105)
flixopt/solvers.py (1)
  • HighsSolver (61-81)
flixopt/plotting.py (1)
  • with_plotly (328-466)
flixopt/modeling.py (4)
flixopt/config.py (1)
  • CONFIG (98-153)
flixopt/structure.py (12)
  • Submodel (757-933)
  • values (991-992)
  • hours_per_step (132-133)
  • hours_per_step (928-929)
  • add_variables (781-790)
  • get_coords (139-168)
  • get_coords (840-845)
  • add_constraints (792-801)
  • constraints (885-891)
  • variables (894-900)
  • get (833-838)
  • get (998-1000)
flixopt/flow_system.py (1)
  • coords (743-749)
flixopt/elements.py (2)
  • previous_states (721-734)
  • previous_states (849-866)
tests/test_storage.py (2)
tests/conftest.py (5)
  • basic_flow_system_linopy_coords (645-660)
  • coords_config (60-62)
  • assert_var_equal (724-758)
  • create_linopy_model (699-702)
  • assert_conequal (705-721)
flixopt/structure.py (6)
  • get_coords (139-168)
  • get_coords (840-845)
  • variables (894-900)
  • constraints (885-891)
  • hours_per_step (132-133)
  • hours_per_step (928-929)
flixopt/aggregation.py (1)
flixopt/structure.py (5)
  • FlowSystemModel (82-200)
  • Submodel (757-933)
  • variables (894-900)
  • add_constraints (792-801)
  • add_variables (781-790)
tests/test_scenarios.py (5)
flixopt/interface.py (5)
  • InvestParameters (663-932)
  • PiecewiseEffects (450-659)
  • Piecewise (83-224)
  • Piece (26-79)
  • PiecewiseConversion (228-446)
flixopt/components.py (1)
  • Storage (235-516)
tests/conftest.py (3)
  • Storage (248-296)
  • create_calculation_and_solve (684-696)
  • create_linopy_model (699-702)
flixopt/flow_system.py (3)
  • FlowSystem (46-879)
  • from_dataset (244-297)
  • sel (755-788)
flixopt/structure.py (7)
  • weights (171-178)
  • label_full (732-733)
  • label_full (871-872)
  • values (991-992)
  • variables (894-900)
  • from_dataset (538-571)
  • do_modeling (98-103)
examples/04_Scenarios/scenario_example.py (3)
flixopt/results.py (5)
  • flow_system (290-307)
  • plot_network (754-774)
  • plot_heatmap (688-752)
  • plot_heatmap (1473-1507)
  • plot_heatmap (1535-1593)
flixopt/flow_system.py (3)
  • FlowSystem (46-879)
  • add_elements (441-465)
  • plot_network (475-512)
flixopt/calculation.py (4)
  • FullCalculation (174-258)
  • do_modeling (182-190)
  • do_modeling (303-317)
  • solve (219-258)
tests/test_dataconverter.py (1)
flixopt/core.py (5)
  • ConversionError (35-38)
  • DataConverter (161-427)
  • TimeSeriesData (41-149)
  • to_dataarray (329-392)
  • aggregation_group (104-105)
tests/test_component.py (2)
tests/conftest.py (9)
  • assert_almost_equal_numeric (669-681)
  • assert_conequal (705-721)
  • assert_sets_equal (761-779)
  • assert_var_equal (724-758)
  • create_calculation_and_solve (684-696)
  • create_linopy_model (699-702)
  • basic_flow_system_linopy_coords (645-660)
  • coords_config (60-62)
  • basic_flow_system (444-459)
flixopt/structure.py (5)
  • variables (894-900)
  • constraints (885-891)
  • get_coords (139-168)
  • get_coords (840-845)
  • values (991-992)
tests/test_functional.py (5)
tests/conftest.py (1)
  • costs (99-100)
flixopt/structure.py (2)
  • solution (106-129)
  • values (991-992)
flixopt/elements.py (3)
  • _investment (709-711)
  • on_off (702-706)
  • flow_rate (621-623)
flixopt/features.py (5)
  • size (121-123)
  • is_invested (126-130)
  • off (259-261)
  • switch_on (264-266)
  • switch_off (269-271)
flixopt/results.py (2)
  • size (1283-1291)
  • flow_rate (1275-1276)
flixopt/interface.py (4)
flixopt/structure.py (4)
  • Interface (203-707)
  • transform_data (221-230)
  • values (991-992)
  • items (985-986)
flixopt/components.py (3)
  • transform_data (207-213)
  • transform_data (423-463)
  • transform_data (694-701)
flixopt/flow_system.py (3)
  • FlowSystem (46-879)
  • fit_to_model_coords (346-395)
  • fit_effects_to_model_coords (397-421)
tests/conftest.py (1)
  • piecewise (203-220)
tests/test_on_hours_computation.py (1)
flixopt/modeling.py (3)
  • ModelingUtilities (101-176)
  • compute_consecutive_hours_in_state (103-125)
  • compute_previous_states (128-129)
flixopt/features.py (6)
flixopt/interface.py (9)
  • InvestParameters (663-932)
  • OnOffParameters (936-1197)
  • Piecewise (83-224)
  • minimum_or_fixed_size (927-928)
  • maximum_or_fixed_size (931-932)
  • use_off (1171-1173)
  • use_switch_on (1186-1197)
  • use_consecutive_on_hours (1176-1178)
  • use_consecutive_off_hours (1181-1183)
flixopt/modeling.py (8)
  • BoundingPatterns (361-587)
  • ModelingPrimitives (179-358)
  • ModelingUtilities (101-176)
  • bounds_with_state (399-442)
  • expression_tracking_variable (183-219)
  • state_transition_bounds (545-587)
  • consecutive_duration_tracking (222-313)
  • compute_consecutive_hours_in_state (103-125)
flixopt/structure.py (14)
  • FlowSystemModel (82-200)
  • Submodel (757-933)
  • _do_modeling (931-933)
  • add_variables (781-790)
  • get_coords (139-168)
  • get_coords (840-845)
  • add_submodels (71-79)
  • add_constraints (792-801)
  • hours_per_step (132-133)
  • hours_per_step (928-929)
  • get (833-838)
  • get (998-1000)
  • values (991-992)
  • variables (894-900)
flixopt/components.py (3)
  • _do_modeling (715-732)
  • _do_modeling (755-791)
  • _do_modeling (802-865)
flixopt/effects.py (4)
  • _do_modeling (233-272)
  • _do_modeling (500-513)
  • effects (411-412)
  • add_share_to_effects (473-493)
flixopt/elements.py (5)
  • _do_modeling (513-542)
  • _do_modeling (745-767)
  • _do_modeling (791-838)
  • previous_states (721-734)
  • previous_states (849-866)
flixopt/structure.py (2)
flixopt/core.py (4)
  • TimeSeriesData (41-149)
  • get_dataarray_stats (430-454)
  • is_timeseries_data (127-129)
  • from_dataarray (112-124)
flixopt/flow_system.py (7)
  • create_model (467-473)
  • coords (743-749)
  • fit_to_model_coords (346-395)
  • FlowSystem (46-879)
  • _create_reference_structure (189-227)
  • to_dataset (229-241)
  • from_dataset (244-297)
flixopt/components.py (4)
flixopt/features.py (8)
  • InvestmentModel (25-130)
  • PiecewiseModel (348-428)
  • _do_modeling (50-53)
  • _do_modeling (162-228)
  • _do_modeling (322-345)
  • _do_modeling (379-428)
  • _do_modeling (453-484)
  • _do_modeling (519-543)
flixopt/modeling.py (2)
  • BoundingPatterns (361-587)
  • scaled_bounds (445-485)
flixopt/structure.py (12)
  • FlowSystemModel (82-200)
  • create_model (728-729)
  • values (991-992)
  • items (985-986)
  • _do_modeling (931-933)
  • add_constraints (792-801)
  • add_submodels (71-79)
  • add_variables (781-790)
  • get_coords (139-168)
  • get_coords (840-845)
  • hours_per_step (132-133)
  • hours_per_step (928-929)
flixopt/flow_system.py (5)
  • create_model (467-473)
  • FlowSystem (46-879)
  • fit_to_model_coords (346-395)
  • flows (734-736)
  • coords (743-749)
flixopt/calculation.py (5)
flixopt/core.py (6)
  • DataConverter (161-427)
  • TimeSeriesData (41-149)
  • drop_constant_arrays (457-470)
  • to_dataarray (329-392)
  • is_timeseries_data (127-129)
  • from_dataarray (112-124)
flixopt/flow_system.py (10)
  • FlowSystem (46-879)
  • used_in_calculation (752-753)
  • sel (755-788)
  • connect_and_transform (423-439)
  • create_model (467-473)
  • to_dataset (229-241)
  • coords (743-749)
  • from_dataset (244-297)
  • _connect_network (620-652)
  • flows (734-736)
flixopt/structure.py (15)
  • FlowSystemModel (82-200)
  • copy (687-699)
  • solution (106-129)
  • values (991-992)
  • label_full (732-733)
  • label_full (871-872)
  • do_modeling (98-103)
  • create_model (728-729)
  • items (985-986)
  • variables (894-900)
  • to_dataset (494-517)
  • weights (171-178)
  • from_dataset (538-571)
  • get (833-838)
  • get (998-1000)
flixopt/utils.py (1)
  • round_nested_floats (16-56)
flixopt/aggregation.py (3)
  • do_modeling (310-338)
  • Aggregation (41-229)
  • cluster (83-107)
flixopt/effects.py (3)
flixopt/features.py (8)
  • ShareAllocationModel (487-588)
  • _do_modeling (50-53)
  • _do_modeling (162-228)
  • _do_modeling (322-345)
  • _do_modeling (379-428)
  • _do_modeling (453-484)
  • _do_modeling (519-543)
  • add_share (545-588)
flixopt/structure.py (20)
  • Element (710-754)
  • ElementModel (1003-1024)
  • FlowSystemModel (82-200)
  • Submodel (757-933)
  • label_full (732-733)
  • label_full (871-872)
  • create_model (728-729)
  • _plausibility_checks (723-726)
  • _do_modeling (931-933)
  • add_submodels (71-79)
  • add_variables (781-790)
  • get_coords (139-168)
  • get_coords (840-845)
  • add_constraints (792-801)
  • values (991-992)
  • items (985-986)
  • weights (171-178)
  • add (994-996)
  • get (833-838)
  • get (998-1000)
flixopt/flow_system.py (4)
  • fit_to_model_coords (346-395)
  • fit_effects_to_model_coords (397-421)
  • create_model (467-473)
  • coords (743-749)
flixopt/elements.py (6)
flixopt/features.py (9)
  • InvestmentModel (25-130)
  • OnOffModel (133-302)
  • size (121-123)
  • _do_modeling (50-53)
  • _do_modeling (162-228)
  • _do_modeling (322-345)
  • _do_modeling (379-428)
  • _do_modeling (453-484)
  • _do_modeling (519-543)
flixopt/modeling.py (9)
  • ModelingPrimitives (179-358)
  • BoundingPatterns (361-587)
  • ModelingUtilitiesAbstract (14-98)
  • expression_tracking_variable (183-219)
  • bounds_with_state (399-442)
  • scaled_bounds (445-485)
  • scaled_bounds_with_state (488-542)
  • to_binary (18-52)
  • mutual_exclusivity_constraint (316-358)
flixopt/interface.py (11)
  • InvestParameters (663-932)
  • OnOffParameters (936-1197)
  • _plausibility_checks (911-924)
  • transform_data (76-79)
  • transform_data (222-224)
  • transform_data (444-446)
  • transform_data (656-659)
  • transform_data (876-909)
  • transform_data (1141-1168)
  • minimum_or_fixed_size (927-928)
  • maximum_or_fixed_size (931-932)
flixopt/structure.py (19)
  • Element (710-754)
  • ElementModel (1003-1024)
  • FlowSystemModel (82-200)
  • create_model (728-729)
  • _plausibility_checks (723-726)
  • transform_data (221-230)
  • label_full (732-733)
  • label_full (871-872)
  • _do_modeling (931-933)
  • add_variables (781-790)
  • get_coords (139-168)
  • get_coords (840-845)
  • hours_per_step (132-133)
  • hours_per_step (928-929)
  • add_submodels (71-79)
  • results_structure (1019-1024)
  • add_constraints (792-801)
  • values (991-992)
  • register_variable (803-811)
flixopt/effects.py (9)
  • effects (411-412)
  • create_model (217-220)
  • create_model (303-306)
  • _plausibility_checks (222-224)
  • _plausibility_checks (358-371)
  • transform_data (177-215)
  • _do_modeling (233-272)
  • _do_modeling (500-513)
  • add_share_to_effects (473-493)
flixopt/flow_system.py (5)
  • create_model (467-473)
  • FlowSystem (46-879)
  • fit_to_model_coords (346-395)
  • fit_effects_to_model_coords (397-421)
  • coords (743-749)
tests/test_effect.py (3)
tests/conftest.py (7)
  • assert_conequal (705-721)
  • assert_sets_equal (761-779)
  • assert_var_equal (724-758)
  • create_calculation_and_solve (684-696)
  • create_linopy_model (699-702)
  • basic_flow_system_linopy_coords (645-660)
  • coords_config (60-62)
flixopt/results.py (8)
  • flow_system (290-307)
  • variables (269-273)
  • variables (838-846)
  • constraints (276-280)
  • constraints (849-857)
  • size (1283-1291)
  • effect_share_factors (283-287)
  • effects_per_component (351-367)
flixopt/structure.py (8)
  • variables (894-900)
  • constraints (885-891)
  • get_coords (139-168)
  • get_coords (840-845)
  • hours_per_step (132-133)
  • hours_per_step (928-929)
  • items (985-986)
  • values (991-992)
flixopt/flow_system.py (4)
flixopt/core.py (5)
  • ConversionError (35-38)
  • DataConverter (161-427)
  • TimeSeriesData (41-149)
  • fit_to_coords (86-101)
  • to_dataarray (329-392)
flixopt/effects.py (8)
  • effects (411-412)
  • Effect (32-224)
  • EffectCollection (290-460)
  • create_effect_values_dict (319-356)
  • transform_data (177-215)
  • create_model (217-220)
  • create_model (303-306)
  • add_effects (308-317)
flixopt/elements.py (8)
  • Flow (224-504)
  • transform_data (100-105)
  • transform_data (192-195)
  • transform_data (420-451)
  • create_model (95-98)
  • create_model (187-190)
  • create_model (415-418)
  • label_full (493-494)
flixopt/structure.py (20)
  • FlowSystemModel (82-200)
  • Interface (203-707)
  • weights (171-178)
  • _create_reference_structure (232-281)
  • to_dataset (494-517)
  • from_dataset (538-571)
  • get (833-838)
  • get (998-1000)
  • _resolve_dataarray_reference (366-397)
  • _resolve_reference_structure (400-456)
  • to_netcdf (519-535)
  • get_structure (594-613)
  • to_json (631-648)
  • values (991-992)
  • transform_data (221-230)
  • create_model (728-729)
  • label_full (732-733)
  • label_full (871-872)
  • keys (988-989)
  • copy (687-699)
tests/conftest.py (3)
flixopt/structure.py (2)
  • FlowSystemModel (82-200)
  • weights (171-178)
flixopt/flow_system.py (2)
  • FlowSystem (46-879)
  • add_elements (441-465)
flixopt/core.py (3)
  • TimeSeriesData (41-149)
  • aggregation_weight (108-109)
  • aggregation_group (104-105)
tests/test_linear_converter.py (4)
tests/conftest.py (4)
  • basic_flow_system_linopy_coords (645-660)
  • coords_config (60-62)
  • assert_conequal (705-721)
  • assert_var_equal (724-758)
flixopt/elements.py (2)
  • flow_rate (621-623)
  • Flow (224-504)
flixopt/structure.py (7)
  • constraints (885-891)
  • variables (894-900)
  • hours_per_step (132-133)
  • hours_per_step (928-929)
  • get_coords (139-168)
  • get_coords (840-845)
  • add_variables (781-790)
flixopt/flow_system.py (1)
  • coords (743-749)
flixopt/results.py (4)
flixopt/flow_system.py (8)
  • FlowSystem (46-879)
  • to_dataset (229-241)
  • flows (734-736)
  • calculate_hours_per_timestep (171-176)
  • from_dataset (244-297)
  • _connect_network (620-652)
  • coords (743-749)
  • sel (755-788)
flixopt/io.py (2)
  • load_dataset_from_netcdf (249-275)
  • save_dataset_to_netcdf (198-246)
flixopt/structure.py (9)
  • to_dataset (494-517)
  • solution (106-129)
  • items (985-986)
  • get (833-838)
  • get (998-1000)
  • from_dataset (538-571)
  • values (991-992)
  • variables (894-900)
  • copy (687-699)
flixopt/effects.py (2)
  • effects (411-412)
  • calculate_effect_share_factors (438-460)
flixopt/core.py (1)
flixopt/flow_system.py (1)
  • coords (743-749)
tests/test_flow.py (4)
tests/conftest.py (8)
  • assert_conequal (705-721)
  • assert_sets_equal (761-779)
  • assert_var_equal (724-758)
  • create_linopy_model (699-702)
  • basic_flow_system_linopy_coords (645-660)
  • coords_config (60-62)
  • costs (99-100)
  • co2 (103-104)
flixopt/structure.py (4)
  • variables (894-900)
  • get_coords (139-168)
  • get_coords (840-845)
  • constraints (885-891)
flixopt/elements.py (2)
  • Flow (224-504)
  • on_off (702-706)
flixopt/flow_system.py (2)
  • coords (743-749)
  • add_elements (441-465)
🪛 markdownlint-cli2 (0.18.1)
CHANGELOG.md

36-36: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4

(MD001, heading-increment)


55-55: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


56-56: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


57-57: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


59-59: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)


60-60: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)


61-61: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)


62-62: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)


65-65: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)


66-66: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)


117-117: Strong style
Expected: asterisk; Actual: underscore

(MD050, strong-style)


117-117: Strong style
Expected: asterisk; Actual: underscore

(MD050, strong-style)


119-119: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)


120-120: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)


121-121: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)

⏰ 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.11)
  • GitHub Check: test (3.13)
  • GitHub Check: test (3.12)
  • GitHub Check: test (3.10)

Comment on lines +195 to +203
if 'flow_system' in kwargs and flow_system_data is None:
flow_system_data = kwargs.pop('flow_system')
warnings.warn(
"The 'flow_system' parameter is deprecated. Use 'flow_system_data' instead."
"Acess is now by '.flow_system_data', while '.flow_system' returns the restored FlowSystem.",
DeprecationWarning,
stacklevel=2,
)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Backward-compat bug: accept old flow_system (object) by converting to dataset

Current fallback assigns a FlowSystem object to flow_system_data, leading to failures later. Convert when needed.

-        if 'flow_system' in kwargs and flow_system_data is None:
-            flow_system_data = kwargs.pop('flow_system')
+        if 'flow_system' in kwargs and flow_system_data is None:
+            old = kwargs.pop('flow_system')
+            try:
+                from .flow_system import FlowSystem as _FS  # local import to avoid cycles
+            except Exception:
+                _FS = None
+            if _FS is not None and isinstance(old, _FS):
+                flow_system_data = old.to_dataset()
+            elif isinstance(old, xr.Dataset):
+                flow_system_data = old
+            else:
+                raise TypeError(
+                    "Deprecated 'flow_system' parameter must be a FlowSystem or an xr.Dataset. "
+                    f'Got {type(old)}'
+                )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if 'flow_system' in kwargs and flow_system_data is None:
flow_system_data = kwargs.pop('flow_system')
warnings.warn(
"The 'flow_system' parameter is deprecated. Use 'flow_system_data' instead."
"Acess is now by '.flow_system_data', while '.flow_system' returns the restored FlowSystem.",
DeprecationWarning,
stacklevel=2,
)
if 'flow_system' in kwargs and flow_system_data is None:
old = kwargs.pop('flow_system')
try:
from .flow_system import FlowSystem as _FS # local import to avoid cycles
except Exception:
_FS = None
if _FS is not None and isinstance(old, _FS):
flow_system_data = old.to_dataset()
elif isinstance(old, xr.Dataset):
flow_system_data = old
else:
raise TypeError(
"Deprecated 'flow_system' parameter must be a FlowSystem or an xr.Dataset. "
f'Got {type(old)}'
)
warnings.warn(
"The 'flow_system' parameter is deprecated. Use 'flow_system_data' instead."
"Acess is now by '.flow_system_data', while '.flow_system' returns the restored FlowSystem.",
DeprecationWarning,
stacklevel=2,
)
🤖 Prompt for AI Agents
In flixopt/results.py around lines 195 to 203, the code assigns a provided
deprecated 'flow_system' object directly to flow_system_data causing later
failures; update the branch so after popping 'flow_system' you detect if the
value is already a dataset-like object and, if not, convert the FlowSystem
object to the expected dataset by calling its conversion method (e.g. call
flow_system.to_dataset() or another available converter), then assign that
resulting dataset to flow_system_data while keeping the DeprecationWarning and
stacklevel intact.

@FBumann FBumann added the v3.0.0 label Sep 23, 2025
* Remove need for timeseries with extra timestep

* Simplify IO of FLowSystem

* Remove parameter timesteps from IO

* Improve Exceptions and Docstrings

* Improve isel sel and resample methods

* Change test

* Bugfix

* Improve

* Improve

* Add test for Storage Bounds

* Add test for Storage Bounds

* CHANGELOG.md

* ruff check

* Improve types

* CHANGELOG.md

* Bugfix in Storage

* Revert changes in example_calculation_types.py

* Revert changes in simple_example.py

* Add convenient access to Elements in FlowSystem

* Get Aggregated Calculation Working

* Segmented running with wrong results

* Use new persistent FLowSystem to create Calculations upfront

* Improve SegmentedCalcualtion

* Improve SegmentedCalcualtion

* Fix SegmentedResults IO

* ruff check

* Update example

* Updated logger essages to use .label_full instead of .label

* Re-add parameters. Use deprecation warning instead

* Update changelog

* Improve warning message

* Merge

* Merge

* Fit scenario weights to model coords when transforming

* Merge

* Removing logic between minimum, maximum and fixed size from InvestParameters

* Remove selected_timesteps

* Improve TypeHints

* New property on InvestParameters for min/max/fixed size

* Move logic for InvestParameters in Transmission to from Model to Interface

* Make transformation of data more hierarchical (Flows after Components)

* Add scenario validation

* Change Transmission to have a "balanced" attribute. Change Tests accordingly

* Improve index validations

* rename method in tests

* Update DataConverter

* Add DataFrame Support back

* Add copy() to DataConverter

* Update fit_to_model_coords to take a list of coords

* Make the DataConverter more universal by accepting a list of coords/dims

* Update DataConverter for n-d arrays

* Update DataConverter for n-d arrays

* Add extra tests for 3-dims

* Add FLowSystemDimension Type

* Revert some logic about the fit_to_model coords

* Adjust FLowSystem IO for scenarios

* BUGFIX: Raise Exception instead of logging

* Change usage of TimeSeriesData

* Adjust logic to handle non scalars

* Adjust logic to _resolve_dataarray_reference into separate method

* Update IO of FlowSystem

* Improve get_coords()

* Adjust FlowSystem init for correct IO

* Add scenario to sel and isel methods, and dont normalize scenario weights

* Improve scenario_weights_handling

* Add warning for not scaled weights

* Update test_scenarios.py

* Improve util method

* Add objective to solution dataset.

* Update handling of scenario_weights update tests

* Ruff check. Fix type hints

* Fix type hints and improve None handling

* Fix coords in AggregatedCalculation

* Improve Error Messages of DataConversion

* Allow multi dim data conversion and broadcasting by length

* Improve DataConverter to handle multi-dim arrays

* Rename methods and remove unused code

* Improve DataConverter by better splitting handling per datatype. Series only matches index (for one dim). Numpy matches shape

* Add test for error handling

* Update scenario example

* Fix Handling of TimeSeriesData

* Improve DataConverter

* Fix resampling of the FlowSystem

* Improve Warning Message

* Add example that leverages resampling

* Add example that leverages resampling adn fixing of Investments

* Add flag to Calculation if its modeled

* Make flag for connected_and_transformed FLowSystem public

* Make Calcualtion Methods return themselfes to make them chainable

* Improve example

* Improve Unreleased CHANGELOG.md

* Add year coord to FlowSystem

* Improve dimension handling

* Change plotting to use an indexer instead

* Change plotting to use an indexer instead

* Use tuples to set dimensions in Models

* Bugfix in validation logic and test

* Improve Errors

* Improve weights handling and rescaling if None

* Fix typehint

* Update Broadcasting in Storage Bounds and improve type hints

* Make .get_model_coords() return an actual xr.Coordinates Object

* Improve get_coords()

* Rename SystemModel to FlowSystemModel

* First steps

* Improve Feature Patterns

* Improve acess to variables via short names

* Improve

* Add naming options to big_m_binary_bounds()

* Fix and improve FLowModeling with Investment

* Improve

* Tyring to improve the Methods for bounding variables in different scenarios

* Improve BoundingPatterns

* Improve BoundingPatterns

* Improve BoundingPatterns

* Fix duration Modeling

* Fix On + Size

* Fix InvestmentModel

* Fix Models

* Update constraint names in test

* Fix OnOffModel for multiple Flows

* Update constraint names in tests

* Simplify

* Improve handling of vars/cons and models

* Revising the basic structure of a class Model

* Revising the basic structure of a class Model

* Simplify and focus more on own Model class

* Update tests

* Improve state computation in ModelingUtilities

* Improve handling of previous flowrates

* Imropove repr and submodel acess

* Update access pattern in tests

* Fix PiecewiseEffects and StorageModel

* Fix StorageModel and Remove PreventSimultaniousUseModel

* Fix Aggregation and SegmentedCalculation

* Update tests

* Loosen precision in tests

* Update test_on_hours_computation.py and some types

* Rename class Model to Submodel

* rename sub_model to submodel everywhere

* rename self.model to self.submodel everywhere

* Rename .model with .submodel if its only a submodel

* Rename .sub_models with .submodels

* Improve repr

* Improve repr

* Include def  do_modeling() into __init__() of models

* Make properties private

* Improve Inheritance of Models

* V3.0.0/plotting (#285)

* Use indexer to reliably plot solutions with and wihtout scenarios/years

* ruff check

* Improve typehints

* Update CHANGELOG.md

* Bugfix from renaming to .submodel

* Bugfix from renaming to .submodel

* Improve indexer in results plotting

* rename register_submodel() to .add_submodels() adn add SUbmodels collection class

* Add nice repr to FlowSystemModel and Submodel

* Bugfix .variables and .constraints

* Add type checks to modeling.py

* Improve assertion in tests

* Improve docstrings and register ElementModels directly in FlowSystemModel

* Improve __repr__()

* ruff check

* Use new method to compare sets in tests

* ruff check

* Update Contribute.md, some dependencies and add pre-commit

* Pre commit hook

* Run Pre-Commit Hook for the first time

* Fix link in README.md

* Update Effect name in tests to be 'costs' instead of 'Costs' Everywhere
Simplify testing by creating a Element Library

* Improve some of the modeling and coord handling

* Add tests with years and scenarios

* Update tests to run with multiple coords

* Fix Effects dataset computation in case of empty effects

* Update Test for multiple dims
Fix Dim order in scaled_bounds_with_state
Bugfix logic in .use_switch_on

* Fix test with multiple dims

* Fix test with multiple dims

* New test

* New test for previous flow_rates

* Add Model for YearAwareInvestments

* Add FlowSystem.years_per_year attribute and "years_of_last_year" parameter to FlowSystem()

* Add YearAwareInvestmentModel

* Add new Interface

* Improve YearAwareInvestmentModel

* Rename and improve

* Move piecewise_effects

* COmbine TImingInvestment into a single interface

* Add model tests for investment

* Add size_changes variables

* Add size_changes variables

* Improve InvestmentModel

* Improve InvestmentModel

* Rename parameters

* remove old code

* Add a duration_in_years to the InvestTimingParameters

* Improve handling of fixed_duration

* Improve validation and make Investment/divestment optional by default

* Rename some vars and improve previous handling

* Add validation for previous size

* Change fit_to_model_coords to work with a Collection of dims

* Improve fit_to_model_coords

* Improve test

* Update transform_data()

* Add new "year of investment" coord to FlowSystem

* Add 'year_of_investment' dimension to FlowSystem

* Improve InvestmentTiming

* Improve InvestmentTiming

* Add specific_effect back

* add effects_by_investment_year back

* Add year_of_investment to FLowSystem.sel()

* Improve Interface

* Handle selection of years properly again

* Temp

* Make ModelingPrimitives.consecutive_duration_tracking() dim-agnostic

* Use new lifetime variable and constraining methods

* Improve Plausibility check

* Improve InvestmentTImingParameters

* Improve weights

* Adjust test

* Remove old classes

* V3.0.0/main fit to model coords improve (#295)

* Change fit_to_model_coords to work with a Collection of dims

* Improve fit_to_model_coords

* ruff format

* Revert changes

* Update type hints

* Revert changes introduced by new Multiperiod Invest parameters

* Improve CHnagelog and docstring of Storage

* Improve Changelog

* Improve InvestmentModel

* Improve InvestmentModel to have 2 cases. One without years and one with

* Improve InvestmentModel to have 2 cases. One without years and one with years. Further, remove investment_scenarios parameter

* Revert some changes regarding Investments

* Typo

* Remove Investment test file (only local testing)

* More reverted changes

* More reverted changes

* Add years_of_last_year to docstring

* Revert change from Investment

* Revert change from Investment

* Remove old todos.txt file

* Fix typos in CHANGELOG.md

* Improve usage of name_prefix to intelligently join with the label

* Ensure IO of years_of_last_year

* Typo

* Typo
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (15)
CHANGELOG.md (5)

38-70: Fix heading levels and list indentation to satisfy markdownlint (MD001/MD007).

  • Use H3 (###) directly under “## [Unreleased]” instead of H4 (####).
  • Fix nested list indentation to 2 spaces.

Example fixes:

-#### Multi-year-investments
+### Multi-year investments
@@
-#### Stochastic modeling
+### Stochastic modeling
@@
-#### Improved Data handling: IO, resampling and more through xarray
+### Improved Data handling: I/O, resampling and more through xarray
@@
-    * NetCDF export/import capabilities for all Interface objects and FlowSystem
+  * NetCDF export/import capabilities for all Interface objects and FlowSystem
-    * JSON export for documentation purposes
+  * JSON export for documentation purposes

33-35: Polish release intro phrasing.

-This Release brings Multi-year-investments and stochastic modeling to flixopt.
-Further, IO methods were improved and resampling and selection of parts of the FlowSystem is now possible.
+This release brings multi-year investments and stochastic modeling to flixopt.
+Furthermore, I/O methods were improved, and resampling and selection of parts of the FlowSystem are now possible.

72-79: Fix typos and clarity in Added section.

-* FlowSystem Restoring: The used FlowSystem is now accessible directly form the results without manual restoring (lazily). All Parameters can be safely accessed anytime after the solve.
+* FlowSystem restoring: The used FlowSystem is now accessible directly from the results without manual restoring (lazily). All parameters can be safely accessed anytime after the solve.

86-88: Grammar fix (“cannot”).

-* FlowSystems can not be shared across multiple Calculations anymore. A copy of the FlowSystem is created instead, making every Calculation independent
+* FlowSystems cannot be shared across multiple Calculations anymore. A copy of the FlowSystem is created instead, making every Calculation independent

119-119: Avoid MD050 false positives by formatting dunder names as code.

-* Added __repr__() for Submodels to easily inspect its content
+* Added `__repr__()` for Submodels to easily inspect its content
flixopt/interface.py (1)

1169-1179: Fix typo in docstring (“whether”).

-        """Determines wether a Variable for SWITCH-ON is needed or not"""
+        """Determines whether a variable for SWITCH-ON is needed or not"""
flixopt/flow_system.py (4)

79-83: Use validated timesteps when creating timesteps_extra.

Avoid bypassing name/ordering validations by using self.timesteps.

-        self.timesteps_extra = self._create_timesteps_with_extra(timesteps, hours_of_last_timestep)
+        self.timesteps_extra = self._create_timesteps_with_extra(self.timesteps, hours_of_last_timestep)

434-441: Typo in warning (“recommended”).

-                f'Scenario weights are not normalized to 1. This is recomended for a better scaled model. '
+                f'Scenario weights are not normalized to 1. This is recommended for a better scaled model. '

472-479: Align attribute naming for created model (model vs submodel).

create_model assigns to self.submodel while the class defines self.model earlier; consider setting both for consistency.

-        self.submodel = FlowSystemModel(self)
-        return self.submodel
+        self.submodel = FlowSystemModel(self)
+        self.model = self.submodel
+        return self.submodel

560-563: Grammar nit (“Can't”).

-            logger.warning('No network app is currently running. Cant stop it')
+            logger.warning("No network app is currently running. Can't stop it")
flixopt/elements.py (1)

487-489: Typo in error message.

-                    f'Different values in different years or scenarios are not yetsupported.'
+                    f'Different values in different years or scenarios are not yet supported.'
flixopt/components.py (2)

202-205: Correct and clarify warning text.

The message contradicts the condition (variable size) and has typos.

-                    logger.warning(
-                        f'Using a FLow with a fixed size ({flow.label_full}) AND a piecewise_conversion '
-                        f'(in {self.label_full}) and variable size is uncommon. Please check if this is intended!'
-                    )
+                    logger.warning(
+                        f'Using a Flow with variable size (InvestParameters without fixed_size) '
+                        f'and a piecewise_conversion in {self.label_full} is uncommon. Please verify intent '
+                        f'({flow.label_full}).'
+                    )

151-154: Fix typos in user-facing Note.

“doest” → “doesn’t”; “THis” → “This”. Also clarify example directionality.

-        Conversion factors define linear relationships.
-        `{flow1: a1, flow2: a2, ...}` leads to `a1×flow_rate1 + a2×flow_rate2 + ... = 0`
-        Unfortunately the current input format doest read intuitively:
-        {"electricity": 1, "H2": 50} means that the electricity_in flow rate is multiplied by 1
-        and the hydrogen_out flow rate is multiplied by 50. THis leads to 50 electricity --> 1 H2.
+        Conversion factors define linear relationships:
+        `{flow1: a1, flow2: a2, ...}` yields `a1×flow_rate1 + a2×flow_rate2 + ... = 0`.
+        Note: The input format may be unintuitive. For example,
+        `{"electricity": 1, "H2": 50}` implies `1×electricity + 50×H2 = 0`,
+        i.e., 50 units of electricity produce 1 unit of H2.
flixopt/effects.py (2)

320-336: Update docstring examples to reflect label keys (not None) and standard-effect requirement.

Examples still show {None: 20}, but implementation returns {standard_effect_label: 20} and requires a standard effect.

-        effect_values_user = 20                             -> {None: 20}
+        effect_values_user = 20                             -> {'<standard_effect_label>': 20}
@@
-            A dictionary with None or Effect as the key, or None if input is None.
+            A dictionary keyed by effect label, or None if input is None.
+            Note: a standard effect must be defined when passing scalars or None labels.

534-603: BFS queue should use deque; avoid O(n²) pops.

Using list.pop(0) is quadratic. Switch to collections.deque for efficiency on large graphs. Behavior unchanged.

+    from collections import deque
@@
-        queue = [(origin, 1, [origin])]
+        queue = deque([(origin, 1, [origin])])
@@
-        while queue:
-            current_domain, factor, path = queue.pop(0)
+        while queue:
+            current_domain, factor, path = queue.popleft()
@@
-                queue.append((target, indirect_factor, new_path))
+                queue.append((target, indirect_factor, new_path))
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between af4a297 and edfe995.

📒 Files selected for processing (10)
  • CHANGELOG.md (1 hunks)
  • flixopt/components.py (13 hunks)
  • flixopt/effects.py (10 hunks)
  • flixopt/elements.py (8 hunks)
  • flixopt/features.py (7 hunks)
  • flixopt/flow_system.py (10 hunks)
  • flixopt/interface.py (10 hunks)
  • flixopt/modeling.py (1 hunks)
  • flixopt/structure.py (6 hunks)
  • tests/todos.txt (0 hunks)
💤 Files with no reviewable changes (1)
  • tests/todos.txt
🧰 Additional context used
🧬 Code graph analysis (8)
flixopt/flow_system.py (4)
flixopt/core.py (5)
  • ConversionError (35-38)
  • DataConverter (161-427)
  • TimeSeriesData (41-149)
  • fit_to_coords (86-101)
  • to_dataarray (329-392)
flixopt/effects.py (5)
  • effects (412-413)
  • Effect (32-225)
  • EffectCollection (291-461)
  • create_effect_values_dict (320-357)
  • transform_data (177-216)
flixopt/elements.py (7)
  • Bus (120-209)
  • Component (31-116)
  • Flow (226-503)
  • transform_data (100-106)
  • transform_data (193-197)
  • transform_data (422-450)
  • label_full (492-493)
flixopt/structure.py (16)
  • Element (715-759)
  • FlowSystemModel (82-204)
  • Interface (207-712)
  • weights (171-182)
  • _create_reference_structure (237-286)
  • items (990-991)
  • to_dataset (499-522)
  • from_dataset (543-576)
  • get (838-843)
  • get (1003-1005)
  • _resolve_dataarray_reference (371-402)
  • _resolve_reference_structure (405-461)
  • values (996-997)
  • transform_data (225-235)
  • label_full (737-738)
  • label_full (876-877)
flixopt/modeling.py (4)
flixopt/config.py (1)
  • CONFIG (98-153)
flixopt/structure.py (12)
  • Submodel (762-938)
  • values (996-997)
  • hours_per_step (132-133)
  • hours_per_step (933-934)
  • add_variables (786-795)
  • get_coords (139-168)
  • get_coords (845-850)
  • add_constraints (797-806)
  • constraints (890-896)
  • variables (899-905)
  • get (838-843)
  • get (1003-1005)
flixopt/flow_system.py (1)
  • coords (748-754)
flixopt/elements.py (2)
  • previous_states (720-733)
  • previous_states (848-865)
flixopt/interface.py (5)
flixopt/effects.py (2)
  • effects (412-413)
  • transform_data (177-216)
flixopt/components.py (3)
  • transform_data (207-214)
  • transform_data (426-467)
  • transform_data (698-702)
flixopt/elements.py (3)
  • transform_data (100-106)
  • transform_data (193-197)
  • transform_data (422-450)
flixopt/flow_system.py (3)
  • FlowSystem (46-888)
  • fit_to_model_coords (364-402)
  • fit_effects_to_model_coords (404-426)
tests/conftest.py (1)
  • piecewise (203-220)
flixopt/structure.py (6)
flixopt/core.py (4)
  • TimeSeriesData (41-149)
  • get_dataarray_stats (430-454)
  • is_timeseries_data (127-129)
  • from_dataarray (112-124)
flixopt/effects.py (6)
  • effects (412-413)
  • create_model (218-221)
  • create_model (304-307)
  • transform_data (177-216)
  • _plausibility_checks (223-225)
  • _plausibility_checks (359-372)
flixopt/components.py (9)
  • create_model (173-176)
  • create_model (421-424)
  • create_model (693-696)
  • transform_data (207-214)
  • transform_data (426-467)
  • transform_data (698-702)
  • _plausibility_checks (178-205)
  • _plausibility_checks (469-520)
  • _plausibility_checks (667-691)
flixopt/elements.py (9)
  • create_model (95-98)
  • create_model (188-191)
  • create_model (417-420)
  • transform_data (100-106)
  • transform_data (193-197)
  • transform_data (422-450)
  • _plausibility_checks (115-116)
  • _plausibility_checks (199-205)
  • _plausibility_checks (452-489)
flixopt/flow_system.py (8)
  • create_model (472-478)
  • coords (748-754)
  • fit_to_model_coords (364-402)
  • FlowSystem (46-888)
  • _create_reference_structure (206-244)
  • to_dataset (246-258)
  • to_netcdf (317-331)
  • from_dataset (261-315)
flixopt/io.py (4)
  • update (322-330)
  • save_dataset_to_netcdf (198-246)
  • load_dataset_from_netcdf (249-275)
  • remove_none_and_empty (20-34)
flixopt/features.py (5)
flixopt/modeling.py (8)
  • BoundingPatterns (366-735)
  • ModelingPrimitives (179-363)
  • ModelingUtilities (101-176)
  • bounds_with_state (404-447)
  • expression_tracking_variable (183-219)
  • state_transition_bounds (550-592)
  • consecutive_duration_tracking (222-318)
  • compute_consecutive_hours_in_state (103-125)
flixopt/structure.py (15)
  • FlowSystemModel (82-204)
  • Submodel (762-938)
  • _do_modeling (936-938)
  • add_variables (786-795)
  • get_coords (139-168)
  • get_coords (845-850)
  • add_submodels (71-79)
  • items (990-991)
  • add_constraints (797-806)
  • hours_per_step (132-133)
  • hours_per_step (933-934)
  • get (838-843)
  • get (1003-1005)
  • values (996-997)
  • variables (899-905)
flixopt/interface.py (10)
  • InvestParameters (663-914)
  • OnOffParameters (918-1179)
  • Piecewise (83-224)
  • minimum_or_fixed_size (909-910)
  • maximum_or_fixed_size (913-914)
  • items (435-442)
  • use_off (1153-1155)
  • use_switch_on (1168-1179)
  • use_consecutive_on_hours (1158-1160)
  • use_consecutive_off_hours (1163-1165)
flixopt/components.py (3)
  • _do_modeling (716-733)
  • _do_modeling (756-792)
  • _do_modeling (803-866)
flixopt/effects.py (4)
  • _do_modeling (234-273)
  • _do_modeling (501-514)
  • effects (412-413)
  • add_share_to_effects (474-494)
flixopt/components.py (7)
flixopt/core.py (1)
  • PlausibilityError (29-32)
flixopt/elements.py (17)
  • Component (31-116)
  • ComponentModel (783-865)
  • Flow (226-503)
  • create_model (95-98)
  • create_model (188-191)
  • create_model (417-420)
  • _plausibility_checks (115-116)
  • _plausibility_checks (199-205)
  • _plausibility_checks (452-489)
  • label_full (492-493)
  • transform_data (100-106)
  • transform_data (193-197)
  • transform_data (422-450)
  • _do_modeling (512-541)
  • _do_modeling (744-766)
  • _do_modeling (790-837)
  • on_off (701-705)
flixopt/features.py (9)
  • InvestmentModel (24-130)
  • PiecewiseModel (352-432)
  • size (121-123)
  • _do_modeling (50-53)
  • _do_modeling (162-232)
  • _do_modeling (326-349)
  • _do_modeling (383-432)
  • _do_modeling (457-488)
  • _do_modeling (523-547)
flixopt/interface.py (17)
  • InvestParameters (663-914)
  • OnOffParameters (918-1179)
  • transform_data (76-79)
  • transform_data (222-224)
  • transform_data (444-446)
  • transform_data (656-659)
  • transform_data (874-906)
  • transform_data (1123-1150)
  • has_time_dim (198-199)
  • has_time_dim (202-205)
  • has_time_dim (426-427)
  • has_time_dim (430-433)
  • has_time_dim (646-647)
  • has_time_dim (650-654)
  • items (435-442)
  • minimum_or_fixed_size (909-910)
  • maximum_or_fixed_size (913-914)
flixopt/modeling.py (2)
  • BoundingPatterns (366-735)
  • scaled_bounds (450-490)
flixopt/structure.py (14)
  • FlowSystemModel (82-204)
  • create_model (733-734)
  • _plausibility_checks (728-731)
  • label_full (737-738)
  • label_full (876-877)
  • transform_data (225-235)
  • values (996-997)
  • items (990-991)
  • _do_modeling (936-938)
  • add_constraints (797-806)
  • add_submodels (71-79)
  • add_variables (786-795)
  • get_coords (139-168)
  • get_coords (845-850)
flixopt/flow_system.py (5)
  • create_model (472-478)
  • FlowSystem (46-888)
  • fit_to_model_coords (364-402)
  • flows (739-741)
  • coords (748-754)
flixopt/elements.py (6)
flixopt/features.py (9)
  • InvestmentModel (24-130)
  • OnOffModel (133-306)
  • size (121-123)
  • _do_modeling (50-53)
  • _do_modeling (162-232)
  • _do_modeling (326-349)
  • _do_modeling (383-432)
  • _do_modeling (457-488)
  • _do_modeling (523-547)
flixopt/modeling.py (9)
  • ModelingPrimitives (179-363)
  • BoundingPatterns (366-735)
  • ModelingUtilitiesAbstract (14-98)
  • expression_tracking_variable (183-219)
  • bounds_with_state (404-447)
  • scaled_bounds (450-490)
  • scaled_bounds_with_state (493-547)
  • to_binary (18-52)
  • mutual_exclusivity_constraint (321-363)
flixopt/interface.py (10)
  • InvestParameters (663-914)
  • OnOffParameters (918-1179)
  • transform_data (76-79)
  • transform_data (222-224)
  • transform_data (444-446)
  • transform_data (656-659)
  • transform_data (874-906)
  • transform_data (1123-1150)
  • minimum_or_fixed_size (909-910)
  • maximum_or_fixed_size (913-914)
flixopt/structure.py (18)
  • Element (715-759)
  • ElementModel (1008-1029)
  • FlowSystemModel (82-204)
  • create_model (733-734)
  • transform_data (225-235)
  • label_full (737-738)
  • label_full (876-877)
  • _do_modeling (936-938)
  • add_variables (786-795)
  • get_coords (139-168)
  • get_coords (845-850)
  • hours_per_step (132-133)
  • hours_per_step (933-934)
  • add_submodels (71-79)
  • results_structure (1024-1029)
  • add_constraints (797-806)
  • values (996-997)
  • register_variable (808-816)
flixopt/effects.py (8)
  • effects (412-413)
  • create_model (218-221)
  • create_model (304-307)
  • transform_data (177-216)
  • _do_modeling (234-273)
  • _do_modeling (501-514)
  • add_share_to_effects (474-494)
  • add_share_to_penalty (496-499)
flixopt/flow_system.py (5)
  • create_model (472-478)
  • FlowSystem (46-888)
  • fit_to_model_coords (364-402)
  • fit_effects_to_model_coords (404-426)
  • coords (748-754)
flixopt/effects.py (3)
flixopt/features.py (8)
  • ShareAllocationModel (491-592)
  • _do_modeling (50-53)
  • _do_modeling (162-232)
  • _do_modeling (326-349)
  • _do_modeling (383-432)
  • _do_modeling (457-488)
  • _do_modeling (523-547)
  • add_share (549-592)
flixopt/structure.py (20)
  • Element (715-759)
  • ElementModel (1008-1029)
  • FlowSystemModel (82-204)
  • Submodel (762-938)
  • label_full (737-738)
  • label_full (876-877)
  • create_model (733-734)
  • _plausibility_checks (728-731)
  • _do_modeling (936-938)
  • add_submodels (71-79)
  • add_variables (786-795)
  • get_coords (139-168)
  • get_coords (845-850)
  • add_constraints (797-806)
  • values (996-997)
  • items (990-991)
  • weights (171-182)
  • add (999-1001)
  • get (838-843)
  • get (1003-1005)
flixopt/flow_system.py (5)
  • FlowSystem (46-888)
  • fit_to_model_coords (364-402)
  • fit_effects_to_model_coords (404-426)
  • create_model (472-478)
  • coords (748-754)
🪛 markdownlint-cli2 (0.18.1)
CHANGELOG.md

38-38: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4

(MD001, heading-increment)


57-57: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


58-58: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


59-59: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


61-61: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)


62-62: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)


63-63: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)


64-64: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)


67-67: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)


68-68: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)


119-119: Strong style
Expected: asterisk; Actual: underscore

(MD050, strong-style)


119-119: Strong style
Expected: asterisk; Actual: underscore

(MD050, strong-style)


121-121: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)


122-122: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)


123-123: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)

⏰ 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.11)
  • GitHub Check: test (3.10)
  • GitHub Check: test (3.12)
  • GitHub Check: test (3.13)
🔇 Additional comments (23)
flixopt/interface.py (3)

71-80: Piece dims logic looks sound.

Using dims=None when has_time_dim=True and restricting to ['year', 'scenario'] otherwise keeps time-invariant piece bounds non-temporal. Good.


222-225: Propagate has_time_dim from Piecewise to pieces.

Setter correctly cascades to contained pieces; prevents mismatched transforms later. Nice.


874-896: Good: InvestParameters effects/sizes aligned to ['year','scenario'].

Non-temporal investment values map to planning axes; piecewise_effects correctly set to non-time dims.

flixopt/modeling.py (6)

98-126: Type-check hours_per_timestep and reuse epsilon default.

Consider early-return on invalid hours_per_timestep types (already present) and pass epsilon to count_consecutive_states consistently. Current usage is fine after to_binary/count fixes.


74-99: Fix logic in count_consecutive_states (spurious .all(), DataArray vs ndarray).

  • .all() on a bool is invalid.
  • Use ndarray ops consistently.
-        binary_values = binary_values.any(dim=[d for d in binary_values.dims if d != dim])
+        if binary_values.ndim > 1:
+            binary_values = binary_values.any(dim=[d for d in binary_values.dims if d != dim])
@@
-        if np.isclose(binary_values.isel({dim: -1}).item(), 0, atol=epsilon).all():
+        if np.isclose(binary_values.isel({dim: -1}).item(), 0.0, atol=epsilon):
             return 0.0
@@
-        is_zero = np.isclose(binary_values, 0, atol=epsilon)
+        arr = binary_values.values
+        is_zero = np.isclose(arr, 0.0, atol=epsilon)
@@
-        zero_indices = np.where(is_zero)[0]
+        zero_indices = np.where(is_zero)[0]
@@
-        consecutive_values = binary_values.isel({dim: slice(start_idx, None)})
-
-        return float(consecutive_values.sum().item())  # TODO: Som only over one dim?
+        return float(arr[start_idx:].sum())

310-315: Avoid ambiguous truth-value with xr.DataArray in consecutive_duration_tracking.

Comparing with an xr.DataArray inside a Python “and” raises. Convert to scalars first.

-            if previous_duration > 0 and previous_duration < minimum_duration.isel({duration_dim: 0}).max():
+            prev = float(previous_duration)
+            min0 = float(minimum_duration.isel({duration_dim: 0}).max().item())
+            if prev > 0 and prev < min0:
                 constraints['initial_lb'] = model.add_constraints(
                     state_variable.isel({duration_dim: 0}) == 1, name=f'{duration.name}|initial_lb'
                 )

438-446: Correct equality detection in bounds_with_state.

np.all(lower - upper) < ... compares a boolean to float.

-        if np.all(lower_bound - upper_bound) < 1e-10:
+        if np.all(np.isclose(lower_bound, upper_bound, atol=1e-10)):
             fix_constraint = model.add_constraints(variable == variable_state * upper_bound, name=f'{name}|fix')
             return [fix_constraint]

484-486: Correct equality detection in scaled_bounds.

-        if np.abs(rel_lower - rel_upper).all() < 10e-10:
+        if np.all(np.isclose(rel_lower, rel_upper, atol=1e-10)):
             return [model.add_constraints(variable == scaling_variable * rel_lower, name=f'{name}|fixed')]

34-52: Handle scalars/empty arrays in to_binary and guard reduction dims.

Current code errors on scalars (len(values)) and on empty .item() access; also collapse step should guard dims.

-        if not isinstance(values, xr.DataArray):
-            values = xr.DataArray(values, dims=['time'], coords={'time': range(len(values))})
+        if not isinstance(values, xr.DataArray):
+            if np.isscalar(values):
+                values = xr.DataArray(values)
+            else:
+                values = xr.DataArray(values, dims=['time'], coords={'time': range(len(values))})
@@
-        if values.size == 0:
-            return xr.DataArray(0) if values.item() < epsilon else xr.DataArray(1)
+        if values.size == 0:
+            return xr.zeros_like(values, dtype=int)
@@
-            binary_states = binary_states.any(dim=[d for d in binary_states.dims if d not in dims])
+            reduce_dims = [d for d in binary_states.dims if d not in dims]
+            if reduce_dims:
+                binary_states = binary_states.any(dim=reduce_dims)
flixopt/flow_system.py (1)

659-667: Nice: concise repr with connection status.

Helpful for quick diagnostics.

flixopt/elements.py (2)

568-607: Constraint composition is correct across on/off/investment cases.

Good use of BoundingPatterns and Submodel wiring.


472-478: Fix .any() on boolean scalar (use np.any).

.any() on a Python bool raises when relative_minimum is scalar.

-        if (self.relative_minimum > 0).any() and self.on_off_parameters is None:
+        if np.any(self.relative_minimum > 0) and self.on_off_parameters is None:
flixopt/features.py (2)

64-77: Investment sizing/binary bounds wiring looks solid.

bounds_with_state on (min,max) with is_invested cleanly enforces optional investments.


165-204: On/off tracking and switch counting: good use of primitives.

State transition bounds and switch-on totals are consistent; previous state handling is correct.

flixopt/structure.py (2)

171-183: Weights defaulting and normalization are sensible.

Good fallback to years_per_year and normalization.


655-676: Robust repr for Interface; truncation and fallbacks are helpful.

Nice debugging ergonomics.

flixopt/components.py (3)

165-171: LinearConverter temporal data handling looks good.

Type changes to TemporalDataUser and the transform path via fit_to_model_coords are consistent with model-coords. The conversion_factors transformation to xr.DataArray per-flow is appropriate.

Please confirm all callers updated to pass time-aligned data for conversion_factors and that piecewise_conversion.has_time_dim=True is intended for this component.

Also applies to: 207-215, 216-229


832-838: Fix discharge efficiency direction in storage balance (still incorrect).

Docstring uses “- discharge / eta_discharge”, but code multiplies by eta_discharge, overstating discharge. Replace multiply with divide.

-            + charge_rate * eff_charge * hours_per_step
-            - discharge_rate * eff_discharge * hours_per_step,
+            + charge_rate * eff_charge * hours_per_step
+            - discharge_rate * hours_per_step / eff_discharge,

728-733: ._investment is a deprecated alias — not a runtime bug; prefer .investment.
._investment is defined (returns self.investment) in elements.py and components.py, so these calls won't raise at runtime; still replace usages in flixopt/components.py (lines ~728–733 and ~861–867) with .investment for clarity.

Likely an incorrect or invalid review comment.

flixopt/effects.py (4)

177-217: Effect.transform_data aligns with model coords.

Converts temporal scalars/ranges and effect-share dicts using fit_to_model_coords/fit_effects_to_model_coords with correct dims. Looks solid.


231-274: Submodel decomposition for EffectModel is clean.

Invest/operation shares separated with appropriate dims and bounds; total variable aggregates correctly.


496-500: Validate linopy.LinearExpression actually exposes .ndim.

If .ndim isn’t guaranteed on expressions, this TypeError guard could itself raise. Consider using a safer check (e.g., hasattr(expression, "ndim")) or validating shapes at construction time.


439-462: Cycle detection and share-factor expansion: logic reads correctly.

Computes (origin,target) shares and validates cycles pre-modeling; good guardrails for misconfigured cross-effects.

Also applies to: 361-373

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (7)
.github/workflows/python-app.yaml (7)

8-9: Clarify PR branch filter: pull_request.branches matches base (target) branch, not the source.

Including feature/v3 here only runs CI for PRs targeting that branch. If the intent was to ensure PRs from the feature/v3/* source branches run when targeting main/dev, it’s redundant—those are already covered by main/dev as bases. Simplify, or add head-branch gating at job-level if needed.

Option A (simplify):

-  pull_request:
-    branches: [main, dev, feature/v3]
+  pull_request:
+    branches: [main, dev]

Option B (gate certain jobs by source branch):

   test:
     runs-on: ubuntu-24.04
+    if: ${{ github.event_name != 'pull_request' || startsWith(github.head_ref, 'feature/') }}

Also applies to: 3-14


4-6: Consider running CI on pushes to dev as well.

Push triggers are limited to main. If dev is an integration branch, enable push CI there too.

 on:
   push:
-    branches: [main]        # Only main branch
+    branches: [main, dev]

152-157: Guard extract_release_notes.py usage; provide fallback.

If the script is missing or errors, the release job fails before creating the release.

-      - name: Extract release notes
-        run: |
-          VERSION=${GITHUB_REF#refs/tags/v}
-          echo "Extracting release notes for version: $VERSION"
-          python scripts/extract_release_notes.py $VERSION > current_release_notes.md
+      - name: Extract release notes (with fallback)
+        run: |
+          set -euo pipefail
+          VERSION=${GITHUB_REF#refs/tags/v}
+          echo "Extracting release notes for version: $VERSION"
+          if [ -f scripts/extract_release_notes.py ]; then
+            python scripts/extract_release_notes.py "$VERSION" > current_release_notes.md || true
+          fi
+          if [ ! -s current_release_notes.md ]; then
+            echo "Using autogenerated release notes as fallback." > current_release_notes.md
+          fi

202-209: Adopt PyPI Trusted Publishing (OIDC) to remove API tokens.

Replace Twine + long‑lived tokens with pypa/gh-action-pypi-publish. Improves security and reduces secret management.

Add OIDC permissions:

 publish-pypi:
   name: Publish to PyPI
   runs-on: ubuntu-24.04
-  needs: [publish-testpypi]
+  needs: [publish-testpypi]
   if: github.event_name == 'push' && startsWith(github.ref, 'refs/tags/v')
   environment:
     name: pypi
     url: https://pypi.org/p/flixopt
+  permissions:
+    id-token: write

Replace upload steps:

-      - name: Upload to TestPyPI
-        run: |
-          twine upload --repository-url https://test.pypi.org/legacy/ dist/* --verbose --skip-existing
-        env:
-          TWINE_USERNAME: __token__
-          TWINE_PASSWORD: ${{ secrets.TEST_PYPI_API_TOKEN }}
-          TWINE_NON_INTERACTIVE: "1"
+      - name: Publish to TestPyPI (OIDC)
+        uses: pypa/gh-action-pypi-publish@release/v1
+        with:
+          repository-url: https://test.pypi.org/legacy/
+          skip-existing: true
-      - name: Upload to PyPI
-        run: |
-          twine upload dist/* --verbose --skip-existing
-        env:
-          TWINE_USERNAME: __token__
-          TWINE_PASSWORD: ${{ secrets.PYPI_API_TOKEN }}
-          TWINE_NON_INTERACTIVE: "1"
+      - name: Publish to PyPI (OIDC)
+        uses: pypa/gh-action-pypi-publish@release/v1
+        with:
+          skip-existing: true

Note: Configure PyPI/TestPyPI trusted publishers for this repo before merging.

Also applies to: 288-295, 256-264, 168-178, 130-133


169-178: Verify presence of required secrets or switch to OIDC.

If you stay with Twine, ensure TEST_PYPI_API_TOKEN exists, else job will fail. OIDC removes this need.

Also applies to: 202-209


112-118: Add dependency vulnerability scan (supply chain).

Complement Bandit (code) with dependency audit via pip-audit or osv-scanner.

       - name: Run Bandit security scan
         run: |
           # Gate on HIGH severity & MEDIUM confidence; produce JSON artifact
           uvx bandit -r . -x tests,docs -c pyproject.toml -f json -o bandit-report.json -q --severity-level high --confidence-level medium
           # Human-readable output without affecting job status
           uvx bandit -r flixopt/ -c pyproject.toml -q --exit-zero
+
+      - name: Run dependency vulnerability audit
+        run: |
+          uvx pip-audit -r requirements.txt || true
+          uvx pip-audit -s --generate SBOM:cyclonedx-json > sbom.json || true
+
+      - name: Upload SBOM
+        uses: actions/upload-artifact@v4
+        if: always()
+        with:
+          name: sbom
+          path: sbom.json
+          retention-days: 30

33-43: Pin GitHub Actions to commit SHAs for supply-chain hardening.

Using floating major tags (e.g., @v5, @v6) can be replaced by digest-pinned SHAs to reduce risk.

Also applies to: 75-83, 101-111, 136-144, 141-146, 170-178, 183-191, 206-209, 265-273, 269-274, 355-363

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between edfe995 and 2f2d083.

📒 Files selected for processing (1)
  • .github/workflows/python-app.yaml (1 hunks)
⏰ 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.12)
  • GitHub Check: test (3.10)
  • GitHub Check: test (3.11)
🔇 Additional comments (1)
.github/workflows/python-app.yaml (1)

112-118: No change required — flixopt/ exists at repo root

pyproject and the repo contain flixopt/, so uvx bandit -r flixopt/ will scan the package.

* Fix typo

* Prefer robust scalar extraction for timestep sizes in aggregation

* Improve docs and error messages

* Update examples

* Use validated timesteps

* Remove unnessesary import

* Use FlowSystem.model instead of FlowSystem.submodel

* Fix Error message

* Improve CHANGELOG.md

* Use self.standard_effect instead of provate self._standard_effect and update docstring

* in calculate_all_conversion_paths, use `collections.deque` for efficiency on large graphs

* Make aggregation_parameters.hours_per_period more robust by using rounding

* Improve import and typos

* Improve docstring

* Use validated timesteps

* Improve error

* Improve warning

* Improve type hint

* Improve CHANGELOG.md: typos, wording and duplicate entries
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (13)
CHANGELOG.md (1)

55-69: Fix MD007 list indentation (2 spaces for nested bullets).

markdownlint flags inconsistent indentation in nested lists. Normalize nested bullets to two spaces.

Apply:

-    * NetCDF export/import capabilities for all Interface objects and FlowSystem
-    * JSON export for documentation purposes
-    * Recursive handling of nested Interface objects
+  * NetCDF export/import capabilities for all Interface objects and FlowSystem
+  * JSON export for documentation purposes
+  * Recursive handling of nested Interface objects
-   * `sel()` and `isel()` methods for temporal data selection
-   * `resample()` method for temporal resampling
-   * `copy()` method to create a copy of a FlowSystem, including all underlying Elements and their data
-   * `__eq__()` method for FlowSystem comparison
+  * `sel()` and `isel()` methods for temporal data selection
+  * `resample()` method for temporal resampling
+  * `copy()` method to create a copy of a FlowSystem, including all underlying Elements and their data
+  * `__eq__()` method for FlowSystem comparison
-   * `get_dataarray_stats()` function for statistical summaries
-   * Enhanced `DataConverter` class with better TimeSeriesData support
+  * `get_dataarray_stats()` function for statistical summaries
+  * Enhanced `DataConverter` class with better TimeSeriesData support
-   * `fit_to_model_coords()` method for data alignment
-   * `fit_effects_to_model_coords()` method for effect data processing
-   * `connect_and_transform()` method replacing several operations
+  * `fit_to_model_coords()` method for data alignment
+  * `fit_effects_to_model_coords()` method for effect data processing
+  * `connect_and_transform()` method replacing several operations

Also applies to: 119-121

flixopt/elements.py (1)

600-606: Prefer self.investment over _investment for clarity.

Avoid private alias; use the public property consistently.

Apply:

-                scaling_variable=self._investment.size,
+                scaling_variable=self.investment.size,
flixopt/results.py (1)

390-399: Guard empty flows when assembling flow_rates.

Older results may lack flows; avoid xr.concat([]) crash and provide a clear error.

Apply:

-        if self._flow_rates is None:
+        if self._flow_rates is None:
+            if not self.flows:
+                raise ValueError('No flow metadata available in results (pre-v2.2.0).')
             self._flow_rates = self._assign_flow_coords(
flixopt/calculation.py (4)

75-85: Deprecation path OK; adjust active_timesteps typing/behavior

active_timesteps can be None; property type hints and return should reflect Optional to avoid misleading annotations.

-    def active_timesteps(self) -> pd.DatetimeIndex:
+    def active_timesteps(self) -> pd.DatetimeIndex | None:

Also applies to: 160-168


99-100: Remove unused _modeled flag

The modeled property derives from self.model; _modeled is dead state and can be removed.

-        self._modeled = False

105-146: Guard main_results access when model/solutions missing

main_results assumes a solved model. Consider a quick guard to raise a clear error if self.model is None or unsolved to prevent AttributeError in logging paths.


559-564: Optional: Python version dependency (zip(..., strict=True))

strict=True requires Python 3.10+. If older versions are supported, drop strict or guard by version.

What Python versions do we support in this project?

flixopt/components.py (2)

642-647: Type hint: on_off_parameters should be Optional

Signature shows default None; expand annotation for clarity.

-        on_off_parameters: OnOffParameters = None,
+        on_off_parameters: OnOffParameters | None = None,

680-692: Message consistency for balanced Transmission

You compare *_or_fixed_size; align error message fields accordingly.

-                    f'Got: {self.in1.size.minimum_size=}, {self.in1.size.maximum_size=}, {self.in1.size.fixed_size=} and '
-                    f'{self.in2.size.minimum_size=}, {self.in2.size.maximum_size=}, {self.in2.size.fixed_size=}.'
+                    f'Got: {self.in1.size.minimum_or_fixed_size=}, {self.in1.size.maximum_or_fixed_size=} and '
+                    f'{self.in2.size.minimum_or_fixed_size=}, {self.in2.size.maximum_or_fixed_size=}.'
flixopt/effects.py (2)

236-264: Dims type vs value: pass list to ShareAllocationModel

ShareAllocationModel expects list-like dims; you pass tuples here. Convert to list for consistency.

-            ShareAllocationModel(
-                model=self._model,
-                dims=('year', 'scenario'),
+            ShareAllocationModel(
+                model=self._model,
+                dims=['year', 'scenario'],
@@
-            ShareAllocationModel(
-                model=self._model,
-                dims=('time', 'year', 'scenario'),
+            ShareAllocationModel(
+                model=self._model,
+                dims=['time', 'year', 'scenario'],

541-610: Conversion-path calculation: watch path explosion

BFS accumulates multiple path factors; for dense graphs this can blow up. Consider limiting path length or deduplicating equivalent paths later if this becomes hot.

If performance becomes an issue, profile calculate_all_conversion_paths on representative graphs.

flixopt/flow_system.py (2)

521-543: Minor: use connect_and_transform for network app too

For consistency, prefer connect_and_transform over private _connect_network here unless avoiding data transforms is intentional.


762-799: Enforce keyword-only args in FlowSystem.sel

  • Add to the sel docstring: “Positional arguments aren’t supported—use time=, year= or scenario=.”
  • In flixopt/calculation.py:550 change
self.flow_system.sel(timesteps_of_segment)

to

self.flow_system.sel(time=timesteps_of_segment)
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between acae94e and f1bd392.

📒 Files selected for processing (11)
  • CHANGELOG.md (2 hunks)
  • examples/03_Calculation_types/example_calculation_types.py (5 hunks)
  • examples/05_Two-stage-optimization/two_stage_optimization.py (1 hunks)
  • flixopt/calculation.py (12 hunks)
  • flixopt/components.py (14 hunks)
  • flixopt/effects.py (9 hunks)
  • flixopt/elements.py (8 hunks)
  • flixopt/flow_system.py (11 hunks)
  • flixopt/interface.py (10 hunks)
  • flixopt/modeling.py (1 hunks)
  • flixopt/results.py (30 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • examples/05_Two-stage-optimization/two_stage_optimization.py
🧰 Additional context used
🧬 Code graph analysis (8)
examples/03_Calculation_types/example_calculation_types.py (3)
flixopt/core.py (3)
  • TimeSeriesData (41-149)
  • aggregation_weight (108-109)
  • aggregation_group (104-105)
flixopt/solvers.py (1)
  • HighsSolver (61-81)
flixopt/plotting.py (1)
  • with_plotly (328-466)
flixopt/modeling.py (4)
flixopt/config.py (1)
  • CONFIG (98-153)
flixopt/structure.py (12)
  • Submodel (762-938)
  • values (996-997)
  • hours_per_step (132-133)
  • hours_per_step (933-934)
  • add_variables (786-795)
  • get_coords (139-168)
  • get_coords (845-850)
  • add_constraints (797-806)
  • constraints (890-896)
  • variables (899-905)
  • get (838-843)
  • get (1003-1005)
flixopt/flow_system.py (1)
  • coords (750-756)
flixopt/elements.py (2)
  • previous_states (720-733)
  • previous_states (848-865)
flixopt/elements.py (7)
flixopt/core.py (1)
  • PlausibilityError (29-32)
flixopt/features.py (9)
  • InvestmentModel (24-130)
  • OnOffModel (133-306)
  • size (121-123)
  • _do_modeling (50-53)
  • _do_modeling (162-232)
  • _do_modeling (326-349)
  • _do_modeling (383-432)
  • _do_modeling (457-488)
  • _do_modeling (523-547)
flixopt/modeling.py (9)
  • ModelingPrimitives (179-363)
  • BoundingPatterns (366-735)
  • ModelingUtilitiesAbstract (14-98)
  • expression_tracking_variable (183-219)
  • bounds_with_state (404-447)
  • scaled_bounds (450-490)
  • scaled_bounds_with_state (493-547)
  • to_binary (18-52)
  • mutual_exclusivity_constraint (321-363)
flixopt/interface.py (10)
  • InvestParameters (663-914)
  • OnOffParameters (918-1179)
  • transform_data (76-79)
  • transform_data (222-224)
  • transform_data (444-446)
  • transform_data (656-659)
  • transform_data (874-906)
  • transform_data (1123-1150)
  • minimum_or_fixed_size (909-910)
  • maximum_or_fixed_size (913-914)
flixopt/structure.py (19)
  • Element (715-759)
  • ElementModel (1008-1029)
  • FlowSystemModel (82-204)
  • create_model (733-734)
  • _plausibility_checks (728-731)
  • transform_data (225-235)
  • label_full (737-738)
  • label_full (876-877)
  • _do_modeling (936-938)
  • add_variables (786-795)
  • get_coords (139-168)
  • get_coords (845-850)
  • hours_per_step (132-133)
  • hours_per_step (933-934)
  • add_submodels (71-79)
  • results_structure (1024-1029)
  • add_constraints (797-806)
  • values (996-997)
  • register_variable (808-816)
flixopt/effects.py (8)
  • effects (419-420)
  • create_model (218-221)
  • create_model (304-307)
  • _plausibility_checks (223-225)
  • _plausibility_checks (366-379)
  • transform_data (177-216)
  • _do_modeling (234-273)
  • _do_modeling (508-521)
flixopt/flow_system.py (5)
  • create_model (474-480)
  • FlowSystem (46-890)
  • fit_to_model_coords (364-402)
  • fit_effects_to_model_coords (404-426)
  • coords (750-756)
flixopt/results.py (4)
flixopt/flow_system.py (8)
  • FlowSystem (46-890)
  • to_dataset (246-258)
  • flows (741-743)
  • calculate_hours_per_timestep (177-182)
  • from_dataset (261-315)
  • _connect_network (627-659)
  • coords (750-756)
  • sel (762-797)
flixopt/io.py (2)
  • load_dataset_from_netcdf (249-275)
  • save_dataset_to_netcdf (198-246)
flixopt/structure.py (9)
  • to_dataset (499-522)
  • solution (106-129)
  • items (990-991)
  • get (838-843)
  • get (1003-1005)
  • from_dataset (543-576)
  • values (996-997)
  • variables (899-905)
  • copy (692-704)
flixopt/effects.py (2)
  • effects (419-420)
  • calculate_effect_share_factors (446-468)
flixopt/interface.py (5)
flixopt/effects.py (2)
  • effects (419-420)
  • transform_data (177-216)
flixopt/components.py (3)
  • transform_data (208-215)
  • transform_data (427-468)
  • transform_data (699-703)
flixopt/elements.py (3)
  • transform_data (100-106)
  • transform_data (193-197)
  • transform_data (422-450)
flixopt/flow_system.py (3)
  • FlowSystem (46-890)
  • fit_to_model_coords (364-402)
  • fit_effects_to_model_coords (404-426)
tests/conftest.py (1)
  • piecewise (203-220)
flixopt/effects.py (3)
flixopt/features.py (8)
  • ShareAllocationModel (491-592)
  • _do_modeling (50-53)
  • _do_modeling (162-232)
  • _do_modeling (326-349)
  • _do_modeling (383-432)
  • _do_modeling (457-488)
  • _do_modeling (523-547)
  • add_share (549-592)
flixopt/structure.py (19)
  • Element (715-759)
  • ElementModel (1008-1029)
  • FlowSystemModel (82-204)
  • Submodel (762-938)
  • label_full (737-738)
  • label_full (876-877)
  • create_model (733-734)
  • _plausibility_checks (728-731)
  • _do_modeling (936-938)
  • add_submodels (71-79)
  • add_variables (786-795)
  • get_coords (139-168)
  • get_coords (845-850)
  • add_constraints (797-806)
  • items (990-991)
  • values (996-997)
  • add (999-1001)
  • get (838-843)
  • get (1003-1005)
flixopt/flow_system.py (5)
  • FlowSystem (46-890)
  • fit_to_model_coords (364-402)
  • fit_effects_to_model_coords (404-426)
  • create_model (474-480)
  • coords (750-756)
flixopt/components.py (5)
flixopt/elements.py (17)
  • Component (31-116)
  • ComponentModel (783-865)
  • Flow (226-503)
  • create_model (95-98)
  • create_model (188-191)
  • create_model (417-420)
  • _plausibility_checks (115-116)
  • _plausibility_checks (199-205)
  • _plausibility_checks (452-489)
  • label_full (492-493)
  • transform_data (100-106)
  • transform_data (193-197)
  • transform_data (422-450)
  • _do_modeling (512-541)
  • _do_modeling (744-766)
  • _do_modeling (790-837)
  • on_off (701-705)
flixopt/features.py (7)
  • PiecewiseModel (352-432)
  • _do_modeling (50-53)
  • _do_modeling (162-232)
  • _do_modeling (326-349)
  • _do_modeling (383-432)
  • _do_modeling (457-488)
  • _do_modeling (523-547)
flixopt/modeling.py (2)
  • BoundingPatterns (366-735)
  • scaled_bounds (450-490)
flixopt/structure.py (14)
  • FlowSystemModel (82-204)
  • create_model (733-734)
  • _plausibility_checks (728-731)
  • label_full (737-738)
  • label_full (876-877)
  • transform_data (225-235)
  • values (996-997)
  • items (990-991)
  • _do_modeling (936-938)
  • add_constraints (797-806)
  • add_submodels (71-79)
  • add_variables (786-795)
  • get_coords (139-168)
  • get_coords (845-850)
flixopt/flow_system.py (5)
  • create_model (474-480)
  • FlowSystem (46-890)
  • fit_to_model_coords (364-402)
  • flows (741-743)
  • coords (750-756)
flixopt/flow_system.py (4)
flixopt/core.py (5)
  • ConversionError (35-38)
  • DataConverter (161-427)
  • TimeSeriesData (41-149)
  • fit_to_coords (86-101)
  • to_dataarray (329-392)
flixopt/effects.py (5)
  • effects (419-420)
  • Effect (32-225)
  • EffectCollection (291-468)
  • create_effect_values_dict (320-364)
  • transform_data (177-216)
flixopt/elements.py (7)
  • Bus (120-209)
  • Component (31-116)
  • Flow (226-503)
  • transform_data (100-106)
  • transform_data (193-197)
  • transform_data (422-450)
  • label_full (492-493)
flixopt/structure.py (16)
  • Element (715-759)
  • FlowSystemModel (82-204)
  • Interface (207-712)
  • weights (171-182)
  • _create_reference_structure (237-286)
  • items (990-991)
  • to_dataset (499-522)
  • from_dataset (543-576)
  • get (838-843)
  • get (1003-1005)
  • _resolve_dataarray_reference (371-402)
  • _resolve_reference_structure (405-461)
  • values (996-997)
  • transform_data (225-235)
  • label_full (737-738)
  • label_full (876-877)
🪛 markdownlint-cli2 (0.18.1)
CHANGELOG.md

57-57: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


58-58: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


59-59: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


61-61: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)


62-62: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)


63-63: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)


64-64: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)


67-67: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)


68-68: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)


119-119: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)


120-120: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)


121-121: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)

⏰ 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.12)
  • GitHub Check: test (3.10)
  • GitHub Check: test (3.13)
  • GitHub Check: test (3.11)
🔇 Additional comments (19)
flixopt/interface.py (1)

197-206: Good: coherent time-dim propagation in Piecewise.

The has_time_dim setter correctly cascades to pieces; transform_data respects dims based on it.

examples/03_Calculation_types/example_calculation_types.py (1)

49-50: Wrong series used for sell price.

Use electricity_price, not electricity_demand.

Apply:

-TS_electricity_price_sell = fx.TimeSeriesData(-(electricity_demand - 0.5), aggregation_group='p_el')
+TS_electricity_price_sell = fx.TimeSeriesData(-(electricity_price - 0.5), aggregation_group='p_el')
flixopt/elements.py (1)

472-477: Fix .any() on scalar boolean (use np.any).

Calling .any() on a scalar raises. Use np.any(...) so both scalars and arrays work.

Apply:

-        if (self.relative_minimum > 0).any() and self.on_off_parameters is None:
+        if np.any(self.relative_minimum > 0) and self.on_off_parameters is None:
flixopt/modeling.py (5)

34-53: Harden to_binary for scalars/empty arrays and robust dim reduction.

Current code errors on scalars and zero-length arrays; dim reduction can over-reduce.

Apply:

-        if not isinstance(values, xr.DataArray):
-            values = xr.DataArray(values, dims=['time'], coords={'time': range(len(values))})
+        if not isinstance(values, xr.DataArray):
+            if np.isscalar(values):
+                values = xr.DataArray(values)
+            else:
+                values = xr.DataArray(values, dims=['time'], coords={'time': range(len(values))})
@@
-        if values.size == 0:
-            return xr.DataArray(0) if values.item() < epsilon else xr.DataArray(1)
+        if values.size == 0:
+            return xr.zeros_like(values, dtype=int)
@@
-        if dims is not None:
-            dims = [dims] if isinstance(dims, str) else dims
-
-            binary_states = binary_states.any(dim=[d for d in binary_states.dims if d not in dims])
+        if dims is not None:
+            dims = [dims] if isinstance(dims, str) else dims
+            reduce_dims = [d for d in binary_states.dims if d not in dims]
+            if reduce_dims:
+                binary_states = binary_states.any(dim=reduce_dims)

74-99: Fix count_consecutive_states scalar/nd handling and boolean misuse.

Avoid .all() on scalar and mix-ups between xarray and numpy arrays.

Apply:

-        binary_values = binary_values.any(dim=[d for d in binary_values.dims if d != dim])
+        if binary_values.ndim > 1:
+            binary_values = binary_values.any(dim=[d for d in binary_values.dims if d != dim])
@@
-        if np.isclose(binary_values.isel({dim: -1}).item(), 0, atol=epsilon).all():
+        if np.isclose(binary_values.isel({dim: -1}).item(), 0.0, atol=epsilon):
             return 0.0
@@
-        is_zero = np.isclose(binary_values, 0, atol=epsilon)
+        arr = binary_values.values
+        is_zero = np.isclose(arr, 0.0, atol=epsilon)
@@
-        zero_indices = np.where(is_zero)[0]
-        if len(zero_indices) == 0:
+        zero_indices = np.where(is_zero)[0]
+        if zero_indices.size == 0:
             # All 'on' - sum everything
             start_idx = 0
         else:
             # Start after last zero
             start_idx = zero_indices[-1] + 1
@@
-        consecutive_values = binary_values.isel({dim: slice(start_idx, None)})
-
-        return float(consecutive_values.sum().item())  # TODO: Som only over one dim?
+        return float(arr[start_idx:].sum())

311-314: Avoid ambiguous truth-value with xr objects in initial min-duration check.

Coerce to scalars before comparison.

Apply:

-            if previous_duration > 0 and previous_duration < minimum_duration.isel({duration_dim: 0}).max():
+            prev = float(previous_duration) if not isinstance(previous_duration, xr.DataArray) else float(previous_duration.max().item())
+            min0 = float(minimum_duration.isel({duration_dim: 0}).max().item())
+            if prev > 0 and prev < min0:

438-446: Correct equality detection in bounds_with_state.

Current boolean-vs-float comparison is wrong; use isclose across arrays.

Apply:

-        if np.all(lower_bound - upper_bound) < 1e-10:
+        if np.all(np.isclose(lower_bound, upper_bound, atol=1e-10)):
             fix_constraint = model.add_constraints(variable == variable_state * upper_bound, name=f'{name}|fix')
             return [fix_constraint]

484-486: Correct equality detection in scaled_bounds.

Use elementwise closeness.

Apply:

-        if np.abs(rel_lower - rel_upper).all() < 10e-10:
+        if np.all(np.isclose(rel_lower, rel_upper, atol=1e-10)):
             return [model.add_constraints(variable == scaling_variable * rel_lower, name=f'{name}|fixed')]
flixopt/results.py (1)

195-203: Backward-compat: convert old flow_system objects to dataset.

Assigning FlowSystem directly breaks later. Convert to xr.Dataset when provided.

Apply:

-        if 'flow_system' in kwargs and flow_system_data is None:
-            flow_system_data = kwargs.pop('flow_system')
+        if 'flow_system' in kwargs and flow_system_data is None:
+            old = kwargs.pop('flow_system')
+            try:
+                from .flow_system import FlowSystem as _FS  # local import to avoid cycles
+            except Exception:
+                _FS = None
+            if _FS is not None and isinstance(old, _FS):
+                flow_system_data = old.to_dataset()
+            elif isinstance(old, xr.Dataset):
+                flow_system_data = old
+            else:
+                raise TypeError(
+                    "Deprecated 'flow_system' parameter must be a FlowSystem or an xr.Dataset. "
+                    f'Got {type(old)}'
+                )
flixopt/calculation.py (4)

68-74: Resolved: safe reuse guard now uses FlowSystem.used_in_calculation

The property exists on FlowSystem; this no longer risks AttributeError. LGTM.


192-218: Fix-sizes helper is useful; minor: skip non-existent vars early

LGTM overall. The existence check and rounding are appropriate.


248-254: Nice: rounded main_results in log output

The use of utils.round_nested_floats before yaml.dump improves readability. LGTM.


546-554: Bug: FlowSystem.sel requires keyword indexers (time=...)

Passing the indexer positionally will fail. Use time=.

-            calc = FullCalculation(f'{self.name}-{segment_name}', self.flow_system.sel(timesteps_of_segment))
+            calc = FullCalculation(
+                f'{self.name}-{segment_name}',
+                self.flow_system.sel(time=timesteps_of_segment),
+            )
flixopt/components.py (3)

758-793: LinearConverter piecewise modeling wiring looks correct

Submodel registration and zero_point usage are coherent. LGTM.


833-839: Storage balance applies discharge efficiency in wrong direction

Per docstring, discharge losses should divide by eta_discharge. Current code multiplies.

-            + charge_rate * eff_charge * hours_per_step
-            - discharge_rate * eff_discharge * hours_per_step,
+            + charge_rate * eff_charge * hours_per_step
+            - discharge_rate * hours_per_step / eff_discharge,

739-746: Guard relative_losses=None and simplify transmission equation

When relative_losses is None, current expression fails. Use default 0 and clearer form.

-        con_transmission = self.add_constraints(
-            out_flow.submodel.flow_rate == -in_flow.submodel.flow_rate * (self.element.relative_losses - 1),
-            short_name=name,
-        )
+        rel_losses = 0 if self.element.relative_losses is None else self.element.relative_losses
+        con_transmission = self.add_constraints(
+            out_flow.submodel.flow_rate == in_flow.submodel.flow_rate * (1 - rel_losses),
+            short_name=name,
+        )
flixopt/effects.py (1)

503-507: Objective construction is sound

Penalty is scalar; objective_effect is weighted over (year, scenario). Summation is consistent. LGTM.

Also applies to: 519-521

flixopt/flow_system.py (2)

758-761: Public flag for calculation reuse

used_in_calculation accessor backed by private flag is a good fit for Calculation’s copy-on-use. LGTM.


333-347: Serialization guard is helpful

Auto connect_and_transform before get_structure reduces surprises. LGTM.

Comment on lines +572 to +579
if i > 0 and self.nr_of_previous_values > 0:
self._transfer_start_values(i)

calculation.do_modeling()
invest_elements = [
model.label_full
for component in self.flow_system.components.values()
for model in component.model.all_sub_models
if isinstance(model, InvestmentModel)
]
if invest_elements:
logger.critical(
f'Investments are not supported in Segmented Calculation! '
f'Following InvestmentModels were found: {invest_elements}'
)

# Warn about Investments, but only in fist run
if i == 0:
invest_elements = [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Validate previous-values transfer vs overlap

If nr_of_previous_values > overlap_timesteps, the slice will be out of range. Fail fast.

     def _transfer_start_values(self, i: int):
         """
         This function gets the last values of the previous solved segment and
         inserts them as start values for the next segment
         """
+        if self.nr_of_previous_values > self.overlap_timesteps:
+            raise ValueError(
+                f'nr_of_previous_values ({self.nr_of_previous_values}) must be <= overlap_timesteps ({self.overlap_timesteps})'
+            )

Also applies to: 605-618

🤖 Prompt for AI Agents
In flixopt/calculation.py around lines 572-579 (and similarly 605-618), the
transfer of previous values uses a slice that will go out of range when
self.nr_of_previous_values > overlap_timesteps; add a guard that validates the
relationship before slicing and fail fast: check if self.nr_of_previous_values
<= overlap_timesteps (or adjust slicing indices) and if not raise a clear
ValueError (or log and exit) indicating invalid configuration, or clamp/adjust
the slice boundaries to valid indices; apply the same validation and corrective
behavior to the second region (605-618).

Separate example testing from other tests by marking them. By default, purest doesn't run the example tests
* Update DataConverter

* Update tests of error messages

* Update tests of error messages

* Update Dataconverter to allow bool values

* fix tests
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (6)
flixopt/core.py (3)

570-575: Time dimension validation: consider allowing PeriodIndex as well.

Many time-indexed workflows use pd.PeriodIndex. Broadening acceptance improves ergonomics without weakening guarantees.

Apply this diff:

-            if dim_name == 'time' and not isinstance(coord_index, pd.DatetimeIndex):
+            if dim_name == 'time' and not isinstance(coord_index, (pd.DatetimeIndex, pd.PeriodIndex)):
                 raise ConversionError(
                     f'Dimension named "time" should use DatetimeIndex for proper '
-                    f'time-series functionality, got {type(coord_index).__name__}'
+                    f'time-series functionality, got {type(coord_index).__name__}'
                 )

445-459: Fix doc examples to reference DataConverter correctly.

Examples use an undefined variable “converter”. Use the class method directly for clarity.

Apply this diff:

-            >>> coords = {'x': pd.Index([1, 2, 3]), 'y': pd.Index(['a', 'b'])}
-            >>> converter.to_dataarray(42, coords)
+            >>> coords = {'x': pd.Index([1, 2, 3]), 'y': pd.Index(['a', 'b'])}
+            >>> DataConverter.to_dataarray(42, coords)
@@
-            >>> series = pd.Series([10, 20, 30], index=[1, 2, 3])
-            >>> converter.to_dataarray(series, coords)
+            >>> series = pd.Series([10, 20, 30], index=[1, 2, 3])
+            >>> DataConverter.to_dataarray(series, coords)
@@
-            >>> array = np.array([[1, 2], [3, 4], [5, 6]])  # Shape (3, 2)
-            >>> converter.to_dataarray(array, coords)
+            >>> array = np.array([[1, 2], [3, 4], [5, 6]])  # Shape (3, 2)
+            >>> DataConverter.to_dataarray(array, coords)

317-339: Permutation search may be costly for many dims.

For large target_dims, 𝑂(P(n,k)) grows fast. You can pre-index target coordinate lengths and prune permutations by mapping each axis length to candidate dims first.

I can provide a length-to-dims prefilter implementation if you want to pursue this now.

tests/test_dataconverter.py (3)

6-10: Remove the inline “adjust import” comment.

Tests should not include TODO-style comments in import lines.

Apply this diff:

-from flixopt.core import (  # Adjust this import to match your project structure
+from flixopt.core import (

519-544: Rename or drop the alias test; it asserts to_dataarray == to_dataarray.

This test is redundant. If you intended to cover a legacy alias (as_dataarray), add that explicitly or rename to reflect idempotence.

Apply this diff to make intent clear:

-class TestAsDataArrayAlias:
-    """Test that to_dataarray works as an alias for to_dataarray."""
+class TestToDataArrayIdempotence:
+    """to_dataarray should be deterministic and idempotent."""
@@
-    def test_to_dataarray_is_alias(self, time_coords, scenario_coords):
-        """to_dataarray should work identically to to_dataarray."""
+    def test_to_dataarray_idempotent(self, time_coords, scenario_coords):
+        """Repeated calls produce the same result."""

If legacy alias support is intended, add tests for as_dataarray calling to_dataarray or mark the alias deprecated.


1261-1263: Add coverage for TimeSeriesData.fit_to_coords and drop_constant_arrays.

Current tests don’t exercise fit_to_coords or constant-array dropping.

I can add:

  • A test ensuring fit_to_coords returns self when coords match and a new instance when broadcasting to extra dims.
  • A test dataset where one variable is constant along time and gets dropped, verifying no ambiguous truth evaluation.
    Want me to open a follow-up PR with these tests?
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f1bd392 and c03e888.

📒 Files selected for processing (5)
  • .github/workflows/python-app.yaml (4 hunks)
  • flixopt/core.py (3 hunks)
  • pyproject.toml (1 hunks)
  • tests/test_dataconverter.py (1 hunks)
  • tests/test_examples.py (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/python-app.yaml
🧰 Additional context used
🧬 Code graph analysis (2)
tests/test_dataconverter.py (1)
flixopt/core.py (5)
  • ConversionError (36-39)
  • DataConverter (162-579)
  • TimeSeriesData (42-150)
  • to_dataarray (406-529)
  • aggregation_group (105-106)
flixopt/core.py (1)
flixopt/flow_system.py (1)
  • coords (750-756)
🔇 Additional comments (3)
tests/test_examples.py (1)

19-19: Marker rename aligns with new test category.

Assigning these example runs to the new examples mark matches the pytest configuration update and keeps the suite tidy.

flixopt/core.py (2)

609-623: Fix ambiguous truth-value when detecting constant arrays.

Comparing DataArrays in an if condition is ambiguous and will raise at runtime.

Apply this diff:

     for name, da in ds.data_vars.items():
         if dim in da.dims:
-            if da.max(dim) == da.min(dim):
-                drop_vars.append(name)
+            spread = da.max(dim) - da.min(dim)
+            # Reduce across remaining dims (if any) and convert to Python bool
+            is_constant = bool(spread.max().values == 0)
+            if is_constant:
+                drop_vars.append(name)
             continue
         elif drop_arrays_without_dim:
             drop_vars.append(name)

Optional: add a tolerance for float noise (e.g., np.isclose) if “near-constant” is desired.


142-150: Deprecated aliases return undefined private attributes. Return attrs or public props instead.

Accessing agg_group/agg_weight currently raises AttributeError.

Apply this diff:

     @property
     def agg_group(self):
         warnings.warn('agg_group is deprecated, use aggregation_group instead', DeprecationWarning, stacklevel=2)
-        return self._aggregation_group
+        return self.attrs.get('aggregation_group')

     @property
     def agg_weight(self):
         warnings.warn('agg_weight is deprecated, use aggregation_weight instead', DeprecationWarning, stacklevel=2)
-        return self._aggregation_weight
+        return self.attrs.get('aggregation_weight')

flixopt/core.py Outdated
Comment on lines 20 to 25
NonTemporalDataUser = int | float | np.integer | np.floating | np.ndarray | pd.Series | pd.DataFrame | xr.DataArray
"""User data which has no time dimension. Internally converted to a Scalar or an xr.DataArray without a time dimension."""

NonTemporalData = xr.DataArray
"""Internally used datatypes for non-temporal data. Can be a Scalar or an xr.DataArray."""

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Docstring/type alias mismatch for NonTemporalData.

Alias is xr.DataArray but docstring says it can be a Scalar. Either update the alias or the docstring. Minimal fix: align the docstring.

Apply this diff:

-NonTemporalData = xr.DataArray
-"""Internally used datatypes for non-temporal data. Can be a Scalar or an xr.DataArray."""
+NonTemporalData = xr.DataArray
+"""Internally used datatype for non-temporal data. Always an xr.DataArray."""
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
NonTemporalDataUser = int | float | np.integer | np.floating | np.ndarray | pd.Series | pd.DataFrame | xr.DataArray
"""User data which has no time dimension. Internally converted to a Scalar or an xr.DataArray without a time dimension."""
NonTemporalData = xr.DataArray
"""Internally used datatypes for non-temporal data. Can be a Scalar or an xr.DataArray."""
NonTemporalDataUser = int | float | np.integer | np.floating | np.ndarray | pd.Series | pd.DataFrame | xr.DataArray
"""User data which has no time dimension. Internally converted to a Scalar or an xr.DataArray without a time dimension."""
NonTemporalData = xr.DataArray
"""Internally used datatype for non-temporal data. Always an xr.DataArray."""
🤖 Prompt for AI Agents
In flixopt/core.py around lines 20 to 25, the type alias NonTemporalData is
declared as xr.DataArray but the docstring states it can be a Scalar or
xr.DataArray; update the docstring to match the alias (or vice versa). Make the
minimal change by editing the docstring to say "Internally used datatype for
non-temporal data: xr.DataArray." so the comment aligns with the alias.

Comment on lines +72 to +74
if (aggregation_group is not None) and (aggregation_weight is not None):
raise ValueError('Use either aggregation_group or aggregation_weight, not both')

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

Don't forbid using aggregation_group and aggregation_weight together (likely unintended).

These metadata are orthogonal. The current check prevents setting both and can break valid use-cases.

Apply this diff if both should be allowed:

-        if (aggregation_group is not None) and (aggregation_weight is not None):
-            raise ValueError('Use either aggregation_group or aggregation_weight, not both')
+        # aggregation_group and aggregation_weight can coexist

🏁 Script executed:

#!/usr/bin/env bash
set -eux

# Locate core.py
echo "Possible paths for core.py:"
fd -t f core.py

# Show context around aggregation_group
echo "Context around aggregation_group in flixopt/core.py:"
rg -nC5 "aggregation_group" flixopt/core.py

# Show context around aggregation_weight
echo "Context around aggregation_weight in flixopt/core.py:"
rg -nC5 "aggregation_weight" flixopt/core.py

Length of output: 9262


🏁 Script executed:

#!/usr/bin/env bash
set -eux

echo "Other references to aggregation_group outside core.py:"
rg -n "aggregation_group" --exclude "flixopt/core.py"

echo "Other references to aggregation_weight outside core.py:"
rg -n "aggregation_weight" --exclude "flixopt/core.py"

Length of output: 582


Allow aggregation_group and aggregation_weight together Drop the ValueError check in core.py init (lines 72–74) since these metadata are orthogonal.

-        if (aggregation_group is not None) and (aggregation_weight is not None):
-            raise ValueError('Use either aggregation_group or aggregation_weight, not both')
+        # aggregation_group and aggregation_weight can coexist
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (aggregation_group is not None) and (aggregation_weight is not None):
raise ValueError('Use either aggregation_group or aggregation_weight, not both')
# aggregation_group and aggregation_weight can coexist
🤖 Prompt for AI Agents
In flixopt/core.py around lines 72–74, the constructor currently raises
ValueError if both aggregation_group and aggregation_weight are provided; remove
that mutual-exclusion check so both metadata can be accepted together. Edit the
__init__ to delete the if-block that raises the ValueError (or invert to no-op)
and ensure any downstream logic that depended on exclusivity treats the two
attributes as orthogonal (e.g., store both on self and update validation/tests
accordingly).

Comment on lines +93 to +101
if self.coords.equals(xr.Coordinates(coords)):
return self

da = DataConverter.to_dataarray(self.data, coords=coords)
return self.__class__(
da,
aggregation_group=self.aggregation_group,
aggregation_weight=self.aggregation_weight,
name=name if name is not None else self.name,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Invalid use of xr.Coordinates and brittle coord equality in fit_to_coords.

xr.Coordinates(...) is not a public constructor; this will fail. Also equality should compare per-dimension coordinate values. Suggest validating/normalizing coords and short-circuiting only when dims+coords match.

Apply this diff:

-        if self.coords.equals(xr.Coordinates(coords)):
-            return self
-
-        da = DataConverter.to_dataarray(self.data, coords=coords)
+        target_coords, target_dims = DataConverter._validate_and_prepare_target_coordinates(coords)
+        # Fast path: same dims and coordinate values
+        if set(self.dims) == set(target_dims) and all(
+            np.array_equal(self.coords[d].values, target_coords[d].values) for d in target_dims
+        ):
+            return self
+
+        da = DataConverter.to_dataarray(self.data, coords=target_coords)
         return self.__class__(
             da,
             aggregation_group=self.aggregation_group,
             aggregation_weight=self.aggregation_weight,
             name=name if name is not None else self.name,
         )

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In flixopt/core.py around lines 93–101, the code incorrectly constructs
xr.Coordinates(...) (not a public API) and performs a brittle overall equality
check; instead, validate and normalize the incoming coords to a canonical
mapping (e.g. mapping from dim->coordinate values), short‑circuit only when the
incoming dims exactly match self.coords.dims and each dimension's coordinate
array is elementwise equal to the corresponding self.coords[dim]; do not call
xr.Coordinates — just compare dims and per‑dimension coordinate values, and if
they differ, call DataConverter.to_dataarray(self.data, coords=coords) and
return the new instance preserving aggregation_group, aggregation_weight and
name logic.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (8)
.github/workflows/python-app.yaml (2)

93-121: test-examples only on tags — consider allowing manual/PR runs

Good to run examples on releases. Recommend also enabling manual runs and/or PR-gated runs (e.g., when examples/** changes) to catch issues earlier.

Minimal change to allow manual runs too:

-    if: startsWith(github.ref, 'refs/tags/v')
+    if: startsWith(github.ref, 'refs/tags/v') || github.event_name == 'workflow_dispatch'

If you want PR runs when examples change, consider paths-filter and a job-level condition; I can draft that if desired.


200-201: Dependency chain for TestPyPI publish — verify intended flow

publish-testpypi depends on test, test-examples, and create-release; all are tag-gated, which is consistent. Note SKIP_TESTPYPI_UPLOAD only skips verification, not upload; name is misleading.

Consider renaming to SKIP_TESTPYPI_VERIFY or actually honoring it to skip upload too.

If keeping name, at least clarify with a comment.

flixopt/elements.py (2)

599-606: Prefer non-deprecated alias for investment.

_investment is marked deprecated; use self.investment.size.

Apply:

-                scaling_variable=self._investment.size,
+                scaling_variable=self.investment.size,

755-761: Use xarray-native multiplication for aligned coords.

np.multiply works but * keeps xarray coords/attrs clearer.

Apply:

-            excess_penalty = np.multiply(self._model.hours_per_step, self.element.excess_penalty_per_flow_hour)
+            excess_penalty = self._model.hours_per_step * self.element.excess_penalty_per_flow_hour
flixopt/components.py (3)

903-905: Tighten charge_state bounds for fixed-size investments.

Use minimum_or_fixed_size/maximum_or_fixed_size to improve bounds tightness and solver performance.

Apply:

-                relative_lower_bound * self.element.capacity_in_flow_hours.minimum_size,
-                relative_upper_bound * self.element.capacity_in_flow_hours.maximum_size,
+                relative_lower_bound * self.element.capacity_in_flow_hours.minimum_or_fixed_size,
+                relative_upper_bound * self.element.capacity_in_flow_hours.maximum_or_fixed_size,

680-692: *Use PlausibilityError and align message with _or_fixed_size.

These are plausibility checks; keep exception types consistent and messages accurate.

Apply:

-            if self.in2 is None:
-                raise ValueError('Balanced Transmission needs InvestParameters in both in-Flows')
+            if self.in2 is None:
+                raise PlausibilityError('Balanced Transmission needs InvestParameters in both in-Flows')
-            if not isinstance(self.in1.size, InvestParameters) or not isinstance(self.in2.size, InvestParameters):
-                raise ValueError('Balanced Transmission needs InvestParameters in both in-Flows')
+            if not isinstance(self.in1.size, InvestParameters) or not isinstance(self.in2.size, InvestParameters):
+                raise PlausibilityError('Balanced Transmission needs InvestParameters in both in-Flows')
             if (self.in1.size.minimum_or_fixed_size > self.in2.size.maximum_or_fixed_size).any() or (
                 self.in1.size.maximum_or_fixed_size < self.in2.size.minimum_or_fixed_size
             ).any():
-                raise ValueError(
-                    f'Balanced Transmission needs compatible minimum and maximum sizes.'
-                    f'Got: {self.in1.size.minimum_size=}, {self.in1.size.maximum_size=}, {self.in1.size.fixed_size=} and '
-                    f'{self.in2.size.minimum_size=}, {self.in2.size.maximum_size=}, {self.in2.size.fixed_size=}.'
-                )
+                raise PlausibilityError(
+                    'Balanced Transmission needs compatible minimum and maximum sizes.'
+                    f' Got: {self.in1.size.minimum_or_fixed_size=}, {self.in1.size.maximum_or_fixed_size=} and '
+                    f'{self.in2.size.minimum_or_fixed_size=}, {self.in2.size.maximum_or_fixed_size=}.'
+                )

862-867: Avoid deprecated alias in balanced-size constraint.

Use .investment.size instead of ._investment.size.

Apply:

-                self.element.charging.submodel._investment.size * 1
-                == self.element.discharging.submodel._investment.size * 1,
+                self.element.charging.submodel.investment.size
+                == self.element.discharging.submodel.investment.size,
flixopt/effects.py (1)

373-380: Consider PlausibilityError for cycle detection failures.

To align with other plausibility checks, raise PlausibilityError rather than ValueError.

Apply:

-            raise ValueError(f'Error: circular operation-shares detected:\n{cycle_str}')
+            raise PlausibilityError(f'Error: circular operation-shares detected:\n{cycle_str}')
@@
-            raise ValueError(f'Error: circular invest-shares detected:\n{cycle_str}')
+            raise PlausibilityError(f'Error: circular invest-shares detected:\n{cycle_str}')
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c03e888 and 7c9f719.

📒 Files selected for processing (5)
  • .github/workflows/python-app.yaml (4 hunks)
  • flixopt/components.py (14 hunks)
  • flixopt/effects.py (9 hunks)
  • flixopt/elements.py (8 hunks)
  • pyproject.toml (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
flixopt/effects.py (3)
flixopt/features.py (8)
  • ShareAllocationModel (491-592)
  • _do_modeling (50-53)
  • _do_modeling (162-232)
  • _do_modeling (326-349)
  • _do_modeling (383-432)
  • _do_modeling (457-488)
  • _do_modeling (523-547)
  • add_share (549-592)
flixopt/structure.py (20)
  • Element (715-759)
  • ElementModel (1008-1029)
  • FlowSystemModel (82-204)
  • Submodel (762-938)
  • label_full (737-738)
  • label_full (876-877)
  • create_model (733-734)
  • _plausibility_checks (728-731)
  • _do_modeling (936-938)
  • add_submodels (71-79)
  • add_variables (786-795)
  • get_coords (139-168)
  • get_coords (845-850)
  • add_constraints (797-806)
  • items (990-991)
  • values (996-997)
  • weights (171-182)
  • add (999-1001)
  • get (838-843)
  • get (1003-1005)
flixopt/flow_system.py (5)
  • FlowSystem (46-890)
  • fit_to_model_coords (364-402)
  • fit_effects_to_model_coords (404-426)
  • create_model (474-480)
  • coords (750-756)
flixopt/components.py (6)
flixopt/elements.py (17)
  • Component (31-116)
  • ComponentModel (783-865)
  • Flow (226-503)
  • create_model (95-98)
  • create_model (188-191)
  • create_model (417-420)
  • _plausibility_checks (115-116)
  • _plausibility_checks (199-205)
  • _plausibility_checks (452-489)
  • label_full (492-493)
  • transform_data (100-106)
  • transform_data (193-197)
  • transform_data (422-450)
  • _do_modeling (512-541)
  • _do_modeling (744-766)
  • _do_modeling (790-837)
  • on_off (701-705)
flixopt/features.py (9)
  • InvestmentModel (24-130)
  • PiecewiseModel (352-432)
  • size (121-123)
  • _do_modeling (50-53)
  • _do_modeling (162-232)
  • _do_modeling (326-349)
  • _do_modeling (383-432)
  • _do_modeling (457-488)
  • _do_modeling (523-547)
flixopt/interface.py (11)
  • InvestParameters (663-914)
  • OnOffParameters (918-1179)
  • transform_data (76-79)
  • transform_data (222-224)
  • transform_data (444-446)
  • transform_data (656-659)
  • transform_data (874-906)
  • transform_data (1123-1150)
  • items (435-442)
  • minimum_or_fixed_size (909-910)
  • maximum_or_fixed_size (913-914)
flixopt/modeling.py (2)
  • BoundingPatterns (366-735)
  • scaled_bounds (450-490)
flixopt/structure.py (14)
  • FlowSystemModel (82-204)
  • create_model (733-734)
  • _plausibility_checks (728-731)
  • label_full (737-738)
  • label_full (876-877)
  • transform_data (225-235)
  • values (996-997)
  • items (990-991)
  • _do_modeling (936-938)
  • add_constraints (797-806)
  • add_submodels (71-79)
  • add_variables (786-795)
  • get_coords (139-168)
  • get_coords (845-850)
flixopt/flow_system.py (5)
  • create_model (474-480)
  • FlowSystem (46-890)
  • fit_to_model_coords (364-402)
  • flows (741-743)
  • coords (750-756)
flixopt/elements.py (6)
flixopt/features.py (9)
  • InvestmentModel (24-130)
  • OnOffModel (133-306)
  • size (121-123)
  • _do_modeling (50-53)
  • _do_modeling (162-232)
  • _do_modeling (326-349)
  • _do_modeling (383-432)
  • _do_modeling (457-488)
  • _do_modeling (523-547)
flixopt/modeling.py (9)
  • ModelingPrimitives (179-363)
  • BoundingPatterns (366-735)
  • ModelingUtilitiesAbstract (14-98)
  • expression_tracking_variable (183-219)
  • bounds_with_state (404-447)
  • scaled_bounds (450-490)
  • scaled_bounds_with_state (493-547)
  • to_binary (18-52)
  • mutual_exclusivity_constraint (321-363)
flixopt/interface.py (10)
  • InvestParameters (663-914)
  • OnOffParameters (918-1179)
  • transform_data (76-79)
  • transform_data (222-224)
  • transform_data (444-446)
  • transform_data (656-659)
  • transform_data (874-906)
  • transform_data (1123-1150)
  • minimum_or_fixed_size (909-910)
  • maximum_or_fixed_size (913-914)
flixopt/structure.py (19)
  • Element (715-759)
  • ElementModel (1008-1029)
  • FlowSystemModel (82-204)
  • create_model (733-734)
  • _plausibility_checks (728-731)
  • transform_data (225-235)
  • label_full (737-738)
  • label_full (876-877)
  • _do_modeling (936-938)
  • add_variables (786-795)
  • get_coords (139-168)
  • get_coords (845-850)
  • hours_per_step (132-133)
  • hours_per_step (933-934)
  • add_submodels (71-79)
  • results_structure (1024-1029)
  • add_constraints (797-806)
  • values (996-997)
  • register_variable (808-816)
flixopt/effects.py (10)
  • effects (419-420)
  • create_model (218-221)
  • create_model (304-307)
  • _plausibility_checks (223-225)
  • _plausibility_checks (366-379)
  • transform_data (177-216)
  • _do_modeling (234-273)
  • _do_modeling (508-521)
  • add_share_to_effects (481-501)
  • add_share_to_penalty (503-506)
flixopt/flow_system.py (5)
  • create_model (474-480)
  • FlowSystem (46-890)
  • fit_to_model_coords (364-402)
  • fit_effects_to_model_coords (404-426)
  • coords (750-756)
⏰ 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.10)
  • GitHub Check: test (3.13)
  • GitHub Check: test (3.11)
  • GitHub Check: test (3.12)
🔇 Additional comments (10)
pyproject.toml (1)

185-185: Fix Windows-incompatible pytest addopts quoting.

Pytest uses shlex.split(..., posix=False) on Windows, so the single quotes survive and turn the marker expression into "\'not"—tests won’t even start. Swap the inner quotes to double quotes so this tokenizes correctly everywhere.

-addopts = "-m 'not examples'"  # Skip examples by default
+addopts = '-m "not examples"'  # Skip examples by default
.github/workflows/python-app.yaml (3)

8-9: Include feature branch in PR triggers — LGTM

Extending PR branches to include feature/v3/main aligns CI with your release work.


88-88: No changes needed for pytest-xdist .[dev] extras include pytest-xdist==3.8.0, so the --numprocesses=auto flag is supported.


161-161: Release gating includes test-examples — LGTM

create-release waits on lint, test, test-examples, and security. This enforces example coverage before publishing.

scripts/extract_release_notes.py exists with a main() entrypoint and __main__ guard.

flixopt/effects.py (1)

519-521: LGTM: Objective formulation (weighted operation + penalty).

Objective uses scenario weights and scalar penalty; consistent with submodel design.

flixopt/elements.py (1)

472-477: Use np.any(...) to avoid ambiguous truth value on xarray/bool.

.any() on a scalar bool or xarray DataArray yields ambiguous truth value in an if. Use np.any(...).

Apply:

-        if (self.relative_minimum > 0).any() and self.on_off_parameters is None:
+        if np.any(self.relative_minimum > 0) and self.on_off_parameters is None:
flixopt/components.py (4)

833-839: Correct discharge efficiency direction (matches docstring and physics).

Discharge losses must divide by η_discharge, not multiply.

Apply:

-            + charge_rate * eff_charge * hours_per_step
-            - discharge_rate * eff_discharge * hours_per_step,
+            + charge_rate * eff_charge * hours_per_step
+            - discharge_rate * hours_per_step / eff_discharge,

739-746: Guard relative_losses=None and simplify transmission equation.

When relative_losses is None you’ll raise; also the current form is harder to read.

Apply:

-        con_transmission = self.add_constraints(
-            out_flow.submodel.flow_rate == -in_flow.submodel.flow_rate * (self.element.relative_losses - 1),
-            short_name=name,
-        )
+        rel_losses = 0 if self.element.relative_losses is None else self.element.relative_losses
+        con_transmission = self.add_constraints(
+            out_flow.submodel.flow_rate == in_flow.submodel.flow_rate * (1 - rel_losses),
+            short_name=name,
+        )

474-505: *Don’t early-return on 'lastValueOfSim'; use _or_fixed_size and PlausibilityError.

Early return skips bounds plausibility; also prefer minimum_or_fixed_size/maximum_or_fixed_size and raise PlausibilityError for out-of-range.

Apply:

-        if isinstance(self.initial_charge_state, str):
-            if self.initial_charge_state != 'lastValueOfSim':
-                raise PlausibilityError(f'initial_charge_state has undefined value: {self.initial_charge_state}')
-            return
+        initial_is_last = isinstance(self.initial_charge_state, str) and self.initial_charge_state == 'lastValueOfSim'
+        if isinstance(self.initial_charge_state, str) and not initial_is_last:
+            raise PlausibilityError(f'initial_charge_state has undefined value: {self.initial_charge_state}')
         if isinstance(self.capacity_in_flow_hours, InvestParameters):
-            if self.capacity_in_flow_hours.fixed_size is None:
-                maximum_capacity = self.capacity_in_flow_hours.maximum_size
-                minimum_capacity = self.capacity_in_flow_hours.minimum_size
-            else:
-                maximum_capacity = self.capacity_in_flow_hours.fixed_size
-                minimum_capacity = self.capacity_in_flow_hours.fixed_size
+            maximum_capacity = self.capacity_in_flow_hours.maximum_or_fixed_size
+            minimum_capacity = self.capacity_in_flow_hours.minimum_or_fixed_size
         else:
             maximum_capacity = self.capacity_in_flow_hours
             minimum_capacity = self.capacity_in_flow_hours
 
         # initial capacity >= allowed min for maximum_size:
         minimum_initial_capacity = maximum_capacity * self.relative_minimum_charge_state.isel(time=0)
         # initial capacity <= allowed max for minimum_size:
         maximum_initial_capacity = minimum_capacity * self.relative_maximum_charge_state.isel(time=0)
 
-        if (self.initial_charge_state > maximum_initial_capacity).any():
-            raise ValueError(
+        if (not initial_is_last) and np.any(self.initial_charge_state > maximum_initial_capacity):
+            raise PlausibilityError(
                 f'{self.label_full}: {self.initial_charge_state=} '
                 f'is above allowed maximum charge_state {maximum_initial_capacity}'
             )
-        if (self.initial_charge_state < minimum_initial_capacity).any():
-            raise ValueError(
+        if (not initial_is_last) and np.any(self.initial_charge_state < minimum_initial_capacity):
+            raise PlausibilityError(
                 f'{self.label_full}: {self.initial_charge_state=} '
                 f'is below allowed minimum charge_state {minimum_initial_capacity}'
             )

506-521: *Balanced storage: use _or_fixed_size for compatibility and PlausibilityError.

Ensure fixed-size investments are handled and error type is consistent.

Apply:

-        if self.balanced:
+        if self.balanced:
             if not isinstance(self.charging.size, InvestParameters) or not isinstance(
                 self.discharging.size, InvestParameters
             ):
                 raise PlausibilityError(
                     f'Balancing charging and discharging Flows in {self.label_full} is only possible with Investments.'
                 )
 
-            if (self.charging.size.minimum_size > self.discharging.size.maximum_size).any() or (
-                self.charging.size.maximum_size < self.discharging.size.minimum_size
+            if (self.charging.size.minimum_or_fixed_size > self.discharging.size.maximum_or_fixed_size).any() or (
+                self.charging.size.maximum_or_fixed_size < self.discharging.size.minimum_or_fixed_size
             ).any():
                 raise PlausibilityError(
                     f'Balancing charging and discharging Flows in {self.label_full} need compatible minimum and maximum sizes.'
-                    f'Got: {self.charging.size.minimum_size=}, {self.charging.size.maximum_size=} and '
-                    f'{self.discharging.size.minimum_size=}, {self.discharging.size.maximum_size=}.'
+                    f'Got: {self.charging.size.minimum_or_fixed_size=}, {self.charging.size.maximum_or_fixed_size=} and '
+                    f'{self.discharging.size.minimum_or_fixed_size=}, {self.discharging.size.maximum_or_fixed_size=}.'
                 )

Comment on lines +833 to +837
ModelingPrimitives.mutual_exclusivity_constraint(
self,
binary_variables=[flow.submodel.on_off.on for flow in self.element.prevent_simultaneous_flows],
short_name='prevent_simultaneous_use',
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Call the correct helper: mutual_exclusivity_constraint is in BoundingPatterns.

The method is defined on BoundingPatterns, not ModelingPrimitives.

Apply:

-            ModelingPrimitives.mutual_exclusivity_constraint(
+            BoundingPatterns.mutual_exclusivity_constraint(
                 self,
                 binary_variables=[flow.submodel.on_off.on for flow in self.element.prevent_simultaneous_flows],
                 short_name='prevent_simultaneous_use',
             )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
ModelingPrimitives.mutual_exclusivity_constraint(
self,
binary_variables=[flow.submodel.on_off.on for flow in self.element.prevent_simultaneous_flows],
short_name='prevent_simultaneous_use',
)
BoundingPatterns.mutual_exclusivity_constraint(
self,
binary_variables=[flow.submodel.on_off.on for flow in self.element.prevent_simultaneous_flows],
short_name='prevent_simultaneous_use',
)
🤖 Prompt for AI Agents
In flixopt/elements.py around lines 833 to 837, the call incorrectly references
ModelingPrimitives.mutual_exclusivity_constraint even though the helper is
defined on BoundingPatterns; replace the qualifier so the code calls
BoundingPatterns.mutual_exclusivity_constraint(self,
binary_variables=[flow.submodel.on_off.on for flow in
self.element.prevent_simultaneous_flows],
short_name='prevent_simultaneous_use'), and if BoundingPatterns is not already
in scope import or reference it appropriately so the method resolves.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (3)
flixopt/elements.py (2)

599-606: Prefer public property over deprecated alias

Use investment instead of _investment for clarity.

             BoundingPatterns.scaled_bounds_with_state(
                 model=self,
                 variable=self.flow_rate,
-                scaling_variable=self._investment.size,
+                scaling_variable=self.investment.size,
                 relative_bounds=self.relative_flow_rate_bounds,
                 scaling_bounds=(self.element.size.minimum_or_fixed_size, self.element.size.maximum_or_fixed_size),
                 variable_state=self.on_off.on,
             )

713-718: Fix docstring: this exposes the Investment submodel, not OnOff

     def investment(self) -> InvestmentModel | None:
-        """OnOff feature"""
+        """Investment submodel"""
         if 'investment' not in self.submodels:
             return None
         return self.submodels['investment']
flixopt/effects.py (1)

359-364: Avoid property access that raises; check backing field instead

Accessing standard_effect here raises when unset; check the private field.

-        if self.standard_effect is None:
+        if self._standard_effect is None:
             raise KeyError(
                 'Scalar effect value provided but no standard effect is configured. '
                 'Either set an effect as is_standard=True or provide a mapping {effect_label: value}.'
             )
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c03e888 and 3c8f713.

📒 Files selected for processing (5)
  • .github/workflows/python-app.yaml (4 hunks)
  • flixopt/components.py (14 hunks)
  • flixopt/effects.py (9 hunks)
  • flixopt/elements.py (8 hunks)
  • pyproject.toml (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/python-app.yaml
🧰 Additional context used
🧬 Code graph analysis (3)
flixopt/components.py (5)
flixopt/elements.py (17)
  • Component (31-116)
  • ComponentModel (783-865)
  • Flow (226-503)
  • create_model (95-98)
  • create_model (188-191)
  • create_model (417-420)
  • _plausibility_checks (115-116)
  • _plausibility_checks (199-205)
  • _plausibility_checks (452-489)
  • label_full (492-493)
  • transform_data (100-106)
  • transform_data (193-197)
  • transform_data (422-450)
  • _do_modeling (512-541)
  • _do_modeling (744-766)
  • _do_modeling (790-837)
  • on_off (701-705)
flixopt/features.py (9)
  • InvestmentModel (24-130)
  • PiecewiseModel (352-432)
  • size (121-123)
  • _do_modeling (50-53)
  • _do_modeling (162-232)
  • _do_modeling (326-349)
  • _do_modeling (383-432)
  • _do_modeling (457-488)
  • _do_modeling (523-547)
flixopt/modeling.py (2)
  • BoundingPatterns (366-735)
  • scaled_bounds (450-490)
flixopt/structure.py (14)
  • FlowSystemModel (82-204)
  • create_model (733-734)
  • _plausibility_checks (728-731)
  • label_full (737-738)
  • label_full (876-877)
  • transform_data (225-235)
  • values (996-997)
  • items (990-991)
  • _do_modeling (936-938)
  • add_constraints (797-806)
  • add_submodels (71-79)
  • add_variables (786-795)
  • get_coords (139-168)
  • get_coords (845-850)
flixopt/flow_system.py (4)
  • create_model (474-480)
  • FlowSystem (46-890)
  • fit_to_model_coords (364-402)
  • coords (750-756)
flixopt/effects.py (3)
flixopt/features.py (8)
  • ShareAllocationModel (491-592)
  • _do_modeling (50-53)
  • _do_modeling (162-232)
  • _do_modeling (326-349)
  • _do_modeling (383-432)
  • _do_modeling (457-488)
  • _do_modeling (523-547)
  • add_share (549-592)
flixopt/structure.py (17)
  • FlowSystemModel (82-204)
  • Submodel (762-938)
  • label_full (737-738)
  • label_full (876-877)
  • create_model (733-734)
  • _do_modeling (936-938)
  • add_submodels (71-79)
  • add_variables (786-795)
  • get_coords (139-168)
  • get_coords (845-850)
  • add_constraints (797-806)
  • items (990-991)
  • values (996-997)
  • weights (171-182)
  • add (999-1001)
  • get (838-843)
  • get (1003-1005)
flixopt/flow_system.py (5)
  • FlowSystem (46-890)
  • fit_to_model_coords (364-402)
  • fit_effects_to_model_coords (404-426)
  • create_model (474-480)
  • coords (750-756)
flixopt/elements.py (7)
flixopt/features.py (9)
  • InvestmentModel (24-130)
  • OnOffModel (133-306)
  • size (121-123)
  • _do_modeling (50-53)
  • _do_modeling (162-232)
  • _do_modeling (326-349)
  • _do_modeling (383-432)
  • _do_modeling (457-488)
  • _do_modeling (523-547)
flixopt/modeling.py (8)
  • ModelingPrimitives (179-363)
  • BoundingPatterns (366-735)
  • ModelingUtilitiesAbstract (14-98)
  • expression_tracking_variable (183-219)
  • bounds_with_state (404-447)
  • scaled_bounds (450-490)
  • scaled_bounds_with_state (493-547)
  • to_binary (18-52)
flixopt/interface.py (10)
  • InvestParameters (663-914)
  • OnOffParameters (918-1179)
  • transform_data (76-79)
  • transform_data (222-224)
  • transform_data (444-446)
  • transform_data (656-659)
  • transform_data (874-906)
  • transform_data (1123-1150)
  • minimum_or_fixed_size (909-910)
  • maximum_or_fixed_size (913-914)
flixopt/structure.py (17)
  • Element (715-759)
  • ElementModel (1008-1029)
  • FlowSystemModel (82-204)
  • create_model (733-734)
  • transform_data (225-235)
  • label_full (737-738)
  • label_full (876-877)
  • _do_modeling (936-938)
  • add_variables (786-795)
  • get_coords (139-168)
  • get_coords (845-850)
  • hours_per_step (132-133)
  • hours_per_step (933-934)
  • add_submodels (71-79)
  • results_structure (1024-1029)
  • add_constraints (797-806)
  • values (996-997)
flixopt/effects.py (8)
  • effects (419-420)
  • create_model (218-221)
  • create_model (304-307)
  • transform_data (177-216)
  • _do_modeling (234-273)
  • _do_modeling (508-521)
  • add_share_to_effects (481-501)
  • add_share_to_penalty (503-506)
flixopt/components.py (9)
  • create_model (173-176)
  • create_model (422-425)
  • create_model (694-697)
  • transform_data (208-215)
  • transform_data (427-468)
  • transform_data (699-703)
  • _do_modeling (717-734)
  • _do_modeling (757-793)
  • _do_modeling (804-867)
flixopt/flow_system.py (5)
  • create_model (474-480)
  • FlowSystem (46-890)
  • fit_to_model_coords (364-402)
  • fit_effects_to_model_coords (404-426)
  • coords (750-756)
⏰ 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)
🔇 Additional comments (6)
pyproject.toml (1)

185-185: Fix Windows-incompatible pytest addopts quoting

shlex.split(..., posix=False) on Windows ignores single quotes, so pytest sees the marker as "'not" and test invocation fails. Use double quotes inside the string so tokenization works cross-platform.

-addopts = "-m 'not examples'"  # Skip examples by default
+addopts = '-m "not examples"'  # Skip examples by default
flixopt/effects.py (1)

519-521: LGTM: Objective formulation with weights and penalty

Objective composes weighted objective effect and scalar penalty correctly.

flixopt/elements.py (1)

472-477: Use np.any instead of .any() on potential scalars

.any() on a scalar boolean raises; prefer np.any(...).

-        if (self.relative_minimum > 0).any() and self.on_off_parameters is None:
+        if np.any(self.relative_minimum > 0) and self.on_off_parameters is None:
flixopt/components.py (3)

833-839: Discharge efficiency applied in wrong direction

Per doc and standard storage balance, discharge losses should divide, not multiply.

         self.add_constraints(
             charge_state.isel(time=slice(1, None))
             == charge_state.isel(time=slice(None, -1)) * ((1 - rel_loss) ** hours_per_step)
             + charge_rate * eff_charge * hours_per_step
-            - discharge_rate * eff_discharge * hours_per_step,
+            - discharge_rate * hours_per_step / eff_discharge,
             short_name='charge_state',
         )

475-505: Don’t early-return on 'lastValueOfSim'; fix capacity handling and error types

Early return skips critical checks. Use *_or_fixed_size and raise PlausibilityError.

-        if isinstance(self.initial_charge_state, str):
-            if self.initial_charge_state != 'lastValueOfSim':
-                raise PlausibilityError(f'initial_charge_state has undefined value: {self.initial_charge_state}')
-            return
+        initial_is_last = isinstance(self.initial_charge_state, str) and self.initial_charge_state == 'lastValueOfSim'
+        if isinstance(self.initial_charge_state, str) and not initial_is_last:
+            raise PlausibilityError(f'initial_charge_state has undefined value: {self.initial_charge_state}')
         if isinstance(self.capacity_in_flow_hours, InvestParameters):
-            if self.capacity_in_flow_hours.fixed_size is None:
-                maximum_capacity = self.capacity_in_flow_hours.maximum_size
-                minimum_capacity = self.capacity_in_flow_hours.minimum_size
-            else:
-                maximum_capacity = self.capacity_in_flow_hours.fixed_size
-                minimum_capacity = self.capacity_in_flow_hours.fixed_size
+            maximum_capacity = self.capacity_in_flow_hours.maximum_or_fixed_size
+            minimum_capacity = self.capacity_in_flow_hours.minimum_or_fixed_size
         else:
             maximum_capacity = self.capacity_in_flow_hours
             minimum_capacity = self.capacity_in_flow_hours
@@
-        if (self.initial_charge_state > maximum_initial_capacity).any():
-            raise ValueError(
+        if (not initial_is_last) and (self.initial_charge_state > maximum_initial_capacity).any():
+            raise PlausibilityError(
                 f'{self.label_full}: {self.initial_charge_state=} '
                 f'is above allowed maximum charge_state {maximum_initial_capacity}'
             )
-        if (self.initial_charge_state < minimum_initial_capacity).any():
-            raise ValueError(
+        if (not initial_is_last) and (self.initial_charge_state < minimum_initial_capacity).any():
+            raise PlausibilityError(
                 f'{self.label_full}: {self.initial_charge_state=} '
                 f'is below allowed minimum charge_state {minimum_initial_capacity}'
             )

739-746: Guard relative_losses=None and simplify transmission equation

When relative_losses is None, current expression fails. Also make the relation explicit.

-        con_transmission = self.add_constraints(
-            out_flow.submodel.flow_rate == -in_flow.submodel.flow_rate * (self.element.relative_losses - 1),
-            short_name=name,
-        )
+        rel_losses = 0 if self.element.relative_losses is None else self.element.relative_losses
+        con_transmission = self.add_constraints(
+            out_flow.submodel.flow_rate == in_flow.submodel.flow_rate * (1 - rel_losses),
+            short_name=name,
+        )
 
         if self.element.absolute_losses is not None:
             con_transmission.lhs += in_flow.submodel.on_off.on * self.element.absolute_losses

Comment on lines +680 to 692
if self.balanced:
if self.in2 is None:
raise ValueError('Balanced Transmission needs InvestParameters in both in-Flows')
if not isinstance(self.in1.size, InvestParameters) or not isinstance(self.in2.size, InvestParameters):
raise ValueError('Balanced Transmission needs InvestParameters in both in-Flows')
if (self.in1.size.minimum_or_fixed_size > self.in2.size.maximum_or_fixed_size).any() or (
self.in1.size.maximum_or_fixed_size < self.in2.size.minimum_or_fixed_size
).any():
raise ValueError(
'Transmission currently does not support separate InvestParameters for Flows. '
'Please use Flow in1. The size of in2 is equal to in1. THis is handled internally'
f'Balanced Transmission needs compatible minimum and maximum sizes.'
f'Got: {self.in1.size.minimum_size=}, {self.in1.size.maximum_size=}, {self.in1.size.fixed_size=} and '
f'{self.in2.size.minimum_size=}, {self.in2.size.maximum_size=}, {self.in2.size.fixed_size=}.'
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Use PlausibilityError and consistent attributes in balanced Transmission checks

Align exception type and messages; use *_or_fixed_size.

-        if self.balanced:
-            if self.in2 is None:
-                raise ValueError('Balanced Transmission needs InvestParameters in both in-Flows')
-            if not isinstance(self.in1.size, InvestParameters) or not isinstance(self.in2.size, InvestParameters):
-                raise ValueError('Balanced Transmission needs InvestParameters in both in-Flows')
-            if (self.in1.size.minimum_or_fixed_size > self.in2.size.maximum_or_fixed_size).any() or (
-                self.in1.size.maximum_or_fixed_size < self.in2.size.minimum_or_fixed_size
-            ).any():
-                raise ValueError(
-                    f'Balanced Transmission needs compatible minimum and maximum sizes.'
-                    f'Got: {self.in1.size.minimum_size=}, {self.in1.size.maximum_size=}, {self.in1.size.fixed_size=} and '
-                    f'{self.in2.size.minimum_size=}, {self.in2.size.maximum_size=}, {self.in2.size.fixed_size=}.'
-                )
+        if self.balanced:
+            if self.in2 is None:
+                raise PlausibilityError('Balanced Transmission requires in2 (two directions).')
+            if not isinstance(self.in1.size, InvestParameters) or not isinstance(self.in2.size, InvestParameters):
+                raise PlausibilityError('Balanced Transmission requires InvestParameters on both in-Flows.')
+            if (self.in1.size.minimum_or_fixed_size > self.in2.size.maximum_or_fixed_size).any() or (
+                self.in1.size.maximum_or_fixed_size < self.in2.size.minimum_or_fixed_size
+            ).any():
+                raise PlausibilityError(
+                    'Balanced Transmission needs compatible minimum and maximum sizes.'
+                    f' Got: {self.in1.size.minimum_or_fixed_size=}, {self.in1.size.maximum_or_fixed_size=} and '
+                    f'{self.in2.size.minimum_or_fixed_size=}, {self.in2.size.maximum_or_fixed_size=}.'
+                )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if self.balanced:
if self.in2 is None:
raise ValueError('Balanced Transmission needs InvestParameters in both in-Flows')
if not isinstance(self.in1.size, InvestParameters) or not isinstance(self.in2.size, InvestParameters):
raise ValueError('Balanced Transmission needs InvestParameters in both in-Flows')
if (self.in1.size.minimum_or_fixed_size > self.in2.size.maximum_or_fixed_size).any() or (
self.in1.size.maximum_or_fixed_size < self.in2.size.minimum_or_fixed_size
).any():
raise ValueError(
'Transmission currently does not support separate InvestParameters for Flows. '
'Please use Flow in1. The size of in2 is equal to in1. THis is handled internally'
f'Balanced Transmission needs compatible minimum and maximum sizes.'
f'Got: {self.in1.size.minimum_size=}, {self.in1.size.maximum_size=}, {self.in1.size.fixed_size=} and '
f'{self.in2.size.minimum_size=}, {self.in2.size.maximum_size=}, {self.in2.size.fixed_size=}.'
)
if self.balanced:
if self.in2 is None:
raise PlausibilityError('Balanced Transmission requires in2 (two directions).')
if not isinstance(self.in1.size, InvestParameters) or not isinstance(self.in2.size, InvestParameters):
raise PlausibilityError('Balanced Transmission requires InvestParameters on both in-Flows.')
if (self.in1.size.minimum_or_fixed_size > self.in2.size.maximum_or_fixed_size).any() or (
self.in1.size.maximum_or_fixed_size < self.in2.size.minimum_or_fixed_size
).any():
raise PlausibilityError(
'Balanced Transmission needs compatible minimum and maximum sizes.'
f' Got: {self.in1.size.minimum_or_fixed_size=}, {self.in1.size.maximum_or_fixed_size=} and '
f'{self.in2.size.minimum_or_fixed_size=}, {self.in2.size.maximum_or_fixed_size=}.'
)
🤖 Prompt for AI Agents
In flixopt/components.py around lines 680 to 692, the balanced Transmission
checks currently raise ValueError and mix attribute names; change the exceptions
to raise PlausibilityError and update the error messages to reference the
consistent *_or_fixed_size attribute names (e.g., minimum_or_fixed_size,
maximum_or_fixed_size, fixed_or_something if applicable) both in the predicate
comparisons and in the f-string details so the reported fields match the checked
attributes; keep the same logical checks but replace ValueError with
PlausibilityError and ensure the f-string prints the *_or_fixed_size attributes
rather than minimum_size/maximum_size.

Comment on lines 199 to 205
def _plausibility_checks(self) -> None:
if self.excess_penalty_per_flow_hour is not None:
zero_penalty = np.all(np.equal(self.excess_penalty_per_flow_hour, 0))
if zero_penalty:
logger.warning(
f'In Bus {self.label}, the excess_penalty_per_flow_hour is 0. Use "None" or a value > 0.'
f'In Bus {self.label_full}, the excess_penalty_per_flow_hour is 0. Use "None" or a value > 0.'
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fix ambiguous truth-value check for xarray DataArray in zero-penalty guard

np.all(...) on an xarray returns a 0-D DataArray; using it in if is ambiguous and can raise. Coerce to a Python bool.

-        if self.excess_penalty_per_flow_hour is not None:
-            zero_penalty = np.all(np.equal(self.excess_penalty_per_flow_hour, 0))
-            if zero_penalty:
+        if self.excess_penalty_per_flow_hour is not None:
+            zero_penalty = bool(np.all(self.excess_penalty_per_flow_hour == 0).item())
+            if zero_penalty:
                 logger.warning(
                     f'In Bus {self.label_full}, the excess_penalty_per_flow_hour is 0. Use "None" or a value > 0.'
                 )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def _plausibility_checks(self) -> None:
if self.excess_penalty_per_flow_hour is not None:
zero_penalty = np.all(np.equal(self.excess_penalty_per_flow_hour, 0))
if zero_penalty:
logger.warning(
f'In Bus {self.label}, the excess_penalty_per_flow_hour is 0. Use "None" or a value > 0.'
f'In Bus {self.label_full}, the excess_penalty_per_flow_hour is 0. Use "None" or a value > 0.'
)
def _plausibility_checks(self) -> None:
if self.excess_penalty_per_flow_hour is not None:
zero_penalty = bool(np.all(self.excess_penalty_per_flow_hour == 0).item())
if zero_penalty:
logger.warning(
f'In Bus {self.label_full}, the excess_penalty_per_flow_hour is 0. Use "None" or a value > 0.'
)
🤖 Prompt for AI Agents
In flixopt/elements.py around lines 199 to 205, the check uses np.all(...) which
returns a 0-D xarray DataArray and its truth value is ambiguous; coerce the
result to a Python bool before using it in the if statement (e.g., evaluate the
DataArray to a scalar via .item() or .values or wrap in bool(...)) so the guard
becomes a proper boolean check and avoids ambiguous truth-value errors.

# Conflicts:
#	CHANGELOG.md
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
CHANGELOG.md (1)

95-108: Fix nested list indentation for markdownlint

markdownlint (MD007) flagged the nested bullet indentation in this block. The inner bullets currently use 3–4 spaces; please adjust them to two spaces so the doc lint passes.

-* IO for all Interfaces and the FlowSystem with round-trip serialization support
-    * NetCDF export/import capabilities for all Interface objects and FlowSystem
-    * JSON export for documentation purposes
-    * Recursive handling of nested Interface objects
-* FlowSystem data manipulation methods
-   * `sel()` and `isel()` methods for temporal data selection
-   * `resample()` method for temporal resampling
-   * `copy()` method to create a copy of a FlowSystem, including all underlying Elements and their data
-   * `__eq__()` method for FlowSystem comparison
-
-* Core data handling improvements
-   * `get_dataarray_stats()` function for statistical summaries
-   * Enhanced `DataConverter` class with better TimeSeriesData support
+* IO for all Interfaces and the FlowSystem with round-trip serialization support
+  * NetCDF export/import capabilities for all Interface objects and FlowSystem
+  * JSON export for documentation purposes
+  * Recursive handling of nested Interface objects
+* FlowSystem data manipulation methods
+  * `sel()` and `isel()` methods for temporal data selection
+  * `resample()` method for temporal resampling
+  * `copy()` method to create a copy of a FlowSystem, including all underlying Elements and their data
+  * `__eq__()` method for FlowSystem comparison
+
+* Core data handling improvements
+  * `get_dataarray_stats()` function for statistical summaries
+  * Enhanced `DataConverter` class with better TimeSeriesData support
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3c8f713 and 0eeb094.

📒 Files selected for processing (2)
  • .github/workflows/python-app.yaml (1 hunks)
  • CHANGELOG.md (1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
CHANGELOG.md

96-96: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


97-97: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


98-98: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


100-100: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)


101-101: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)


102-102: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)


103-103: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)


106-106: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)


107-107: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)


158-158: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)


159-159: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)


160-160: Unordered list indentation
Expected: 2; Actual: 3

(MD007, ul-indent)

⏰ 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.12)
  • GitHub Check: test (3.11)
  • GitHub Check: test (3.10)

Add emojis to CHANGELOG.md
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

♻️ Duplicate comments (5)
CHANGELOG.md (1)

180-181: Remove stray “Until here -->” marker.

Leftover marker breaks formatting and tooling.

-Until here -->
-
+---
pyproject.toml (1)

198-211: Reorder filters so the experimental network warning is actually ignored.

First-match wins: with error::UserWarning:flixopt before the specific ignore, the warning is promoted to error. Move the message-specific ignore above the blanket error.

Apply this diff:

 filterwarnings = [
     # === Default behavior: show all warnings ===
     "default",
 
     # === Treat flixopt warnings as errors (strict mode for our code) ===
     # This ensures we catch deprecations, future changes, and user warnings in our own code
+    "ignore:.*network visualization is still experimental.*:UserWarning:flixopt",
     "error::DeprecationWarning:flixopt",
     "error::FutureWarning:flixopt",
     "error::UserWarning:flixopt",
@@
-    "ignore:.*network visualization is still experimental.*:UserWarning:flixopt",
 ]
examples/03_Calculation_types/example_calculation_types.py (1)

226-230: Stacked vs grouped bars inconsistency (use grouped_bar or drop override)

Currently style='stacked_bar' but barmode='group'. Pick one approach.

-    fx.plotting.with_plotly(
-        pd.DataFrame(get_solutions(calculations, 'costs(temporal)|per_timestep').to_dataframe().sum()).T,
-        style='stacked_bar',
-        title='Total Cost Comparison',
-        ylabel='Costs [€]',
-    ).update_layout(barmode='group').write_html('results/Total Costs.html')
+    fx.plotting.with_plotly(
+        pd.DataFrame(get_solutions(calculations, 'costs(temporal)|per_timestep').to_dataframe().sum()).T,
+        style='grouped_bar',
+        title='Total Cost Comparison',
+        ylabel='Costs [€]',
+    ).write_html('results/Total Costs.html')
examples/04_Scenarios/scenario_example.py (1)

22-25: Weights shape mismatch with (periods, scenarios) → likely runtime error.

FlowSystem.fit_to_model_coords expects weights with dims ['period','scenario']; a 1D array of length 2 won’t align with periods=3, scenarios=2. Provide a 2D array (or a scenario-indexed object if you truly mean scenario-only weights).

Apply this diff for uniform weights over periods×scenarios:

-    flow_system = fx.FlowSystem(timesteps=timesteps, periods=periods, scenarios=scenarios, weights=np.array([0.5, 0.6]))
+    weights = np.full((len(periods), len(scenarios)), 1.0/(len(periods)*len(scenarios)))
+    flow_system = fx.FlowSystem(timesteps=timesteps, periods=periods, scenarios=scenarios, weights=weights)

Alternatively, for scenario-only weights: pass a pandas Series indexed by scenarios and named 'scenario'.

Based on relevant code snippets (connect_and_transform expects dims ['period','scenario']).

examples/05_Two-stage-optimization/two_stage_optimization.py (1)

131-136: Timer reset and 'Dispatch' naming fixed correctly

Start is reset and calculation is named 'Dispatch' as intended.

🧹 Nitpick comments (17)
examples/02_Complex/complex_example.py (1)

17-20: Consider extracting experiment flags and documenting their purpose.

The experiment configuration flags (check_penalty, excess_penalty, use_chp_with_piecewise_conversion, time_indices) control different behaviors, but their interactions and purposes could be clearer. Consider adding brief comments explaining what each experiment tests.

Example:

 # --- Experiment Options ---
-# Configure options for testing various parameters and behaviors
-check_penalty = False
-excess_penalty = 1e5
-use_chp_with_piecewise_conversion = True
-time_indices = None  # Define specific time steps for custom calculations, or use the entire series
+# Configure options for testing various parameters and behaviors
+check_penalty = False  # Set to True to test penalty handling with extreme heat demand
+excess_penalty = 1e5  # Penalty applied when bus balance cannot be satisfied
+use_chp_with_piecewise_conversion = True  # Toggle between simple CHP and piecewise CHP models
+time_indices = None  # Define specific time steps for custom calculations, or None for entire series
flixopt/__init__.py (1)

54-54: Investigate addressing the tsam minimal value warning in flixopt.

The TODO suggests this warning might be fixable in flixopt rather than suppressed. Addressing the root cause would improve code quality and reduce the need for warning filters.

Do you want me to help investigate the root cause of the tsam minimal value warning and generate a solution to address it in flixopt?

docs/index.md (2)

13-13: Align wording with released capability (avoid “active development”).

CHANGELOG says periods/scenarios are introduced in this release. The parenthetical contradicts that and undermines confidence. Suggest removing it or clarifying stability.

-- **Multi-dimensional modeling**: Full support for multi-period investments and scenario-based stochastic optimization (periods and scenarios are in active development)
+- **Multi-dimensional modeling**: Full support for multi-period investments and scenario-based stochastic optimization

19-21: Add authoritative links for tools/standards.

Link “marimo” and “VDI 2067” to improve discoverability.

- - **Interactive tutorials**: Browser-based, reactive tutorials for learning FlixOpt without local installation (marimo)
+ - **Interactive tutorials**: Browser-based, reactive tutorials for learning FlixOpt without local installation ([marimo](https://marimo.io))
- - **Standardized cost calculations**: Align with industry standards (VDI 2067) for CAPEX/OPEX calculations
+ - **Standardized cost calculations**: Align with industry standards ([VDI 2067](https://www.vdi.de/richtlinien/details/vdi-2067-1-wirtschaftlichkeitsberechnung-von-energieversorgungsanlagen-allgemeine-bewertungen)) for CAPEX/OPEX calculations
CHANGELOG.md (3)

134-136: Call out effect-share removal as breaking, and add a migration hint.

Replacing specific_share_to_other_effects_* without deprecation is breaking. Duplicate a concise note under “💥 Breaking Changes” and add a one-line mapping example to guide users.

 ### 💥 Breaking Changes
 ...
+- Effect share parameters removed without deprecation. Migrate:
+  - `specific_share_to_other_effects_operation` → `share_from_temporal` (direction inverted)
+  - `specific_share_to_other_effects_invest` → `share_from_periodic` (direction inverted)
+  - Example: `{ 'CO2': 180 }` means costs “share from” CO2 at 180 €/tCO2.

Also applies to: 84-92


165-165: Unify spelling: “Modeling” vs “Modelling”.

Use one variant consistently (project mostly uses “modeling”).

-- Added new module `.modeling` that contains Modelling primitives and utilities
+- Added new module `.modeling` that contains modeling primitives and utilities

95-97: Consider elevating FlowSystem sharing change to Breaking Changes.

“FlowSystems cannot be shared across multiple Calculations anymore” can break existing workflows relying on identity/sharing semantics. Duplicating this under “💥 Breaking Changes” improves visibility for users scanning for behavioral breaks.

pyproject.toml (1)

52-54: Tighten numexpr lower bound to ≥2.10.0 for NumPy 2.x support
Update the pyproject.toml marker to "numexpr >= 2.10.0, < 2.14; python_version < '3.11'" since official NumPy 2.x support begins with 2.10.0.

examples/03_Calculation_types/example_calculation_types.py (4)

36-38: Parse datetime index at read time

Use parse_dates to avoid the later manual conversion.

-    data_import = pd.read_csv(
-        pathlib.Path(__file__).parent.parent / 'resources' / 'Zeitreihen2020.csv', index_col=0
-    ).sort_index()
+    data_import = pd.read_csv(
+        pathlib.Path(__file__).parent.parent / 'resources' / 'Zeitreihen2020.csv',
+        index_col=0,
+        parse_dates=[0],
+    ).sort_index()

Then you can drop the separate pd.to_datetime on Line 42.


127-129: Consider wrapping gas_price in TimeSeriesData

For consistency and smoother aggregation handling, wrap gas_price; optionally set an aggregation_group if you want prices grouped in aggregation.

Based on learnings

-            fx.Flow('Q_Gas', bus='Gas', size=1000, effects_per_flow_hour={costs.label: gas_price, CO2.label: 0.3})
+            fx.Flow(
+                'Q_Gas',
+                bus='Gas',
+                size=1000,
+                effects_per_flow_hour={costs.label: fx.TimeSeriesData(gas_price), CO2.label: 0.3},
+            )

176-176: DRY solver config

Define the solver once and reuse to avoid duplication and drift.

Example:

solver = fx.solvers.HighsSolver(0.01 / 100, 60)
...
calculation.solve(solver)
...
calculation.do_modeling_and_solve(solver)
...
calculation.solve(solver)

Also applies to: 181-181, 190-190


233-235: Prefer plotting API kwargs over update_layout here

Pass title/xlabel/ylabel directly and keep style named for consistency.

-    fx.plotting.with_plotly(
-        pd.DataFrame([calc.durations for calc in calculations], index=[calc.name for calc in calculations]),
-        'stacked_bar',
-    ).update_layout(title='Duration Comparison', xaxis_title='Calculation type', yaxis_title='Time (s)').write_html(
-        'results/Speed Comparison.html'
-    )
+    fx.plotting.with_plotly(
+        pd.DataFrame([calc.durations for calc in calculations], index=[calc.name for calc in calculations]),
+        style='stacked_bar',
+        title='Duration Comparison',
+        xlabel='Calculation type',
+        ylabel='Time (s)',
+    ).write_html('results/Speed Comparison.html')
examples/04_Scenarios/scenario_example.py (2)

124-131: Remove duplicate plot_heatmap call.

plot_heatmap('CHP(Q_th)|flow_rate') is called twice; drop one.

-    calculation.results.plot_heatmap('CHP(Q_th)|flow_rate')

122-122: Solver gap of 0 can stall; use a small positive gap.

mip_gap=0 may cause long solves or non-termination. Prefer a small gap (e.g., 1e-4) for examples.

-    calculation.solve(fx.solvers.HighsSolver(mip_gap=0, time_limit_seconds=30))
+    calculation.solve(fx.solvers.HighsSolver(mip_gap=1e-4, time_limit_seconds=30))
examples/05_Two-stage-optimization/two_stage_optimization.py (3)

23-27: Guard against missing CSV and make path explicit

Add an exists check for the resource file to fail fast with a clear error.

-    data_import = pd.read_csv(
-        pathlib.Path(__file__).parent.parent / 'resources' / 'Zeitreihen2020.csv', index_col=0
-    ).sort_index()
+    data_path = pathlib.Path(__file__).resolve().parents[1] / 'resources' / 'Zeitreihen2020.csv'
+    if not data_path.exists():
+        raise FileNotFoundError(f'Missing data file: {data_path}')
+    data_import = pd.read_csv(data_path, index_col=0).sort_index()

126-126: Sizing robustness: consider peak‑preserving resample

Default 'mean' can under‑size capacities. Prefer an explicit method like 'max' for the sizing run.

Based on FlowSystem.resample API in the repo.

-    calculation_sizing = fx.FullCalculation('Sizing', flow_system.resample('4h'))
+    calculation_sizing = fx.FullCalculation('Sizing', flow_system.resample('4h', method='max'))

68-69: Avoid duplicate flow labels on the same bus

Multiple flows labeled 'P_el' (CHP, Einspeisung, Stromtarif) can confuse outputs and plots. Optional: disambiguate labels.

-            P_el=fx.Flow('P_el', bus='Strom'),
+            P_el=fx.Flow('P_el_chp', bus='Strom'),
@@
-                fx.Flow(
-                    'P_el', bus='Strom', size=1000, effects_per_flow_hour={'costs': electricity_price + 0.5, 'CO2': 0.3}
-                )
+                fx.Flow(
+                    'P_el_grid_sell', bus='Strom', size=1000,
+                    effects_per_flow_hour={'costs': electricity_price + 0.5, 'CO2': 0.3}
+                )
@@
-                fx.Flow('P_el', bus='Strom', size=1000, effects_per_flow_hour={'costs': electricity_price, 'CO2': 0.3})
+                fx.Flow('P_el_grid_buy', bus='Strom', size=1000,
+                        effects_per_flow_hour={'costs': electricity_price, 'CO2': 0.3})

Also applies to: 107-110, 117-121

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3bc90f4 and 7624191.

⛔ Files ignored due to path filters (1)
  • examples/resources/Zeitreihen2020.csv is excluded by !**/*.csv
📒 Files selected for processing (9)
  • CHANGELOG.md (2 hunks)
  • docs/index.md (1 hunks)
  • examples/02_Complex/complex_example.py (7 hunks)
  • examples/03_Calculation_types/example_calculation_types.py (7 hunks)
  • examples/04_Scenarios/scenario_example.py (1 hunks)
  • examples/05_Two-stage-optimization/two_stage_optimization.py (1 hunks)
  • flixopt/__init__.py (2 hunks)
  • mkdocs.yml (1 hunks)
  • pyproject.toml (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • mkdocs.yml
🧰 Additional context used
🧬 Code graph analysis (5)
pyproject.toml (2)
tests/test_config.py (2)
  • test_change_logging_level_removed (190-200)
  • test_public_api (202-216)
flixopt/components.py (1)
  • __init__ (1117-1142)
examples/02_Complex/complex_example.py (4)
flixopt/config.py (3)
  • CONFIG (61-322)
  • Logging (88-173)
  • apply (208-244)
flixopt/effects.py (1)
  • Effect (32-387)
flixopt/components.py (2)
  • Source (1115-1219)
  • Sink (1223-1342)
flixopt/flow_system.py (1)
  • start_network_app (519-546)
examples/04_Scenarios/scenario_example.py (3)
flixopt/results.py (11)
  • flow_system (290-306)
  • plot_network (754-774)
  • plot_heatmap (688-752)
  • plot_heatmap (1473-1507)
  • plot_heatmap (1535-1593)
  • plot_node_balance_pie (978-1058)
  • plot_node_balance (917-976)
  • node_balance_with_charge_state (1208-1240)
  • plot_charge_state (1134-1206)
  • to_file (776-823)
  • to_file (1509-1532)
flixopt/flow_system.py (2)
  • FlowSystem (46-980)
  • plot_network (480-517)
flixopt/calculation.py (4)
  • FullCalculation (176-266)
  • do_modeling (191-199)
  • do_modeling (311-325)
  • solve (228-266)
examples/05_Two-stage-optimization/two_stage_optimization.py (3)
flixopt/results.py (3)
  • flow_system (290-306)
  • size (1283-1291)
  • sizes (433-460)
flixopt/flow_system.py (4)
  • FlowSystem (46-980)
  • add_elements (440-464)
  • resample (926-976)
  • sel (852-887)
flixopt/calculation.py (5)
  • FullCalculation (176-266)
  • do_modeling (191-199)
  • do_modeling (311-325)
  • solve (228-266)
  • fix_sizes (201-226)
examples/03_Calculation_types/example_calculation_types.py (4)
flixopt/config.py (3)
  • CONFIG (61-322)
  • Logging (88-173)
  • apply (208-244)
flixopt/core.py (3)
  • TimeSeriesData (42-150)
  • aggregation_weight (109-110)
  • aggregation_group (105-106)
flixopt/components.py (2)
  • Sink (1223-1342)
  • Source (1115-1219)
flixopt/plotting.py (1)
  • with_plotly (328-466)
⏰ 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.10)
  • GitHub Check: test (3.13)
  • GitHub Check: test (3.11)
  • GitHub Check: test (3.12)
🔇 Additional comments (21)
examples/02_Complex/complex_example.py (9)

12-14: LGTM! Logging configuration follows new API.

The logging setup correctly uses the new CONFIG.Logging.console and CONFIG.apply() pattern.


46-47: LGTM! Effect definitions align with new API.

The share_from_temporal={'CO2': 0.2} parameter correctly maps 20% of CO2 effects into Costs. The CO2 effect is properly defined and added to the flow system at line 189, satisfying the dependency.


63-66: LGTM! InvestParameters updated to new API.

Field renames are correct:

  • fix_effectseffects_of_investment
  • optionalmandatory (properly inverted: FalseTrue)
  • specific_effectseffects_of_investment_per_size

78-79: LGTM! Time-varying consecutive hours constraint.

The array-based consecutive_on_hours_min allows finer-grained control, with 9 values matching the 9 timesteps. The added consecutive_off_hours_max parameter enhances the operational constraint modeling.


135-136: LGTM! Storage investment parameters updated consistently.

The parameter renames (piecewise_effects_of_investment, mandatory=True) align with the InvestParameters API updates seen elsewhere in the example.


152-159: LGTM! Sink wiring updated to new API.

The change from sink=fx.Flow(...) to inputs=[fx.Flow(...)] follows the new Sink constructor signature, making the input flows explicit and consistent with the v3 API.


165-172: LGTM! Source wiring updated to new API.

The change from source=fx.Flow(...) to outputs=[fx.Flow(...)] aligns with the new Source constructor signature, making the output flows explicit.


178-184: LGTM! Sink wiring consistently updated.

The Stromverkauf Sink follows the same inputs=[fx.Flow(...)] pattern as the other Sink definitions in the file.


193-193: LGTM! Typo corrected.

Comment grammar fixed from "DOes" to "Does".

flixopt/__init__.py (1)

8-12: LGTM! Version handling follows best practices.

The try/except pattern for reading the installed version with a clear development-mode fallback is correct and idiomatic.

docs/index.md (1)

84-87: Image path casing is correct. The file docs/images/architecture_flixOpt.png exists with matching case, so the link will not 404.

pyproject.toml (1)

189-190: Windows-safe pytest addopts quoting looks good.

The switch to outer single quotes with inner double quotes fixes Windows shlex tokenization.

examples/03_Calculation_types/example_calculation_types.py (6)

14-16: Logging init LGTM

Early CONFIG.apply with console=True is fine.


53-55: Good: TimeSeriesData with aggregation metadata

Using aggregation_weight and aggregation_group matches the new API.

Based on learnings


116-122: Verify fixed_relative_profile alignment

Ensure both profiles’ lengths match timesteps and scaling is as intended (profile × size). Mismatches will error or skew loads.


140-141: Sell tariff wiring LGTM

Negative sell price via TimeSeriesData is correct for revenues.


145-152: Buy tariff wiring LGTM

Effect mapping with TimeSeriesData for costs is consistent with the API.


219-221: Costs series plotting LGTM

Variable path and line style align with plotting API.

examples/04_Scenarios/scenario_example.py (1)

80-88: Verify InvestParameters keyword names match current signature. Confirm that effects_of_investment, fixed_size and mandatory are valid parameters on InvestParameters (e.g. via class annotations or runtime introspection); any mismatch will raise at runtime.

examples/05_Two-stage-optimization/two_stage_optimization.py (2)

138-142: Size equality check is sound

Rounding to 5 decimals matches fix_sizes default; boolean reduction to Python bool is correct.


150-177: Verify and correct cost variable names in comparison
Ensure the keys you select from comparison match those in results.solution: if temporal costs are named
'costs(temporal)|per_timestep' (not plain 'costs(temporal)'), update that entry; confirm whether a plain 'costs' variable exists (and drop or rename it if not).

Comment on lines +61 to +63
# Advanced control when needed - extend with native linopy
boiler.model.add_constraints(custom_constraint, name="my_constraint")
```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Snippet likely uses the wrong access path to the underlying linopy model.

In v3, direct .model on a component (e.g., boiler.model) is not part of the public surface. Access the linopy model via Calculation (or FlowSystemModel), e.g., calc = fx.Calculation(fs).do_modeling(); calc.model....

-# Advanced control when needed - extend with native linopy
-boiler.model.add_constraints(custom_constraint, name="my_constraint")
+# Advanced control when needed — extend with native linopy
+calc = fx.Calculation(flow_system).do_modeling()
+calc.model.add_constraints(custom_constraint, name="my_constraint")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Advanced control when needed - extend with native linopy
boiler.model.add_constraints(custom_constraint, name="my_constraint")
```
# Advanced control when needed extend with native linopy
calc = fx.Calculation(flow_system).do_modeling()
calc.model.add_constraints(custom_constraint, name="my_constraint")
🤖 Prompt for AI Agents
In docs/index.md around lines 61 to 63, the snippet uses a non-public access
path (.model on a component like boiler.model); update the example to obtain the
linopy model via the public Calculation/FlowSystemModel API (e.g., create a
Calculation for the FlowSystemModel and run do_modeling() to get the calculation
object) and then call add_constraints on calc.model or the calculation's model
object instead of using component.model directly so the docs reflect the v3
public surface.

Comment on lines 203 to 210
# --- Plotting for comparison ---
fx.plotting.with_plotly(
get_solutions(calculations, 'Speicher|charge_state').to_dataframe(),
mode='line',
style='line',
title='Charge State Comparison',
ylabel='Charge state',
).write_html('results/Charge State.html')

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Create results/ directory before writing HTML

write_html will fail if the folder doesn’t exist.

     # --- Plotting for comparison ---
+    pathlib.Path('results').mkdir(parents=True, exist_ok=True)
     fx.plotting.with_plotly(
         get_solutions(calculations, 'Speicher|charge_state').to_dataframe(),
         style='line',
         title='Charge State Comparison',
         ylabel='Charge state',
     ).write_html('results/Charge State.html')

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In examples/03_Calculation_types/example_calculation_types.py around lines
203–210, the call to write_html('results/Charge State.html') will fail if the
results/ directory does not exist; ensure the directory is created before
calling write_html by creating the results folder (e.g., using os.makedirs or
pathlib.Path(...).mkdir with parents=True and exist_ok=True) just prior to
writing the file so the HTML can be saved without error.

Comment on lines +22 to +23
power_prices = np.array([0.08, 0.09, 0.10])

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Make power price effect explicit and dimensioned by 'period'.

Passing a raw ndarray to effects_per_flow_hour is ambiguous and currently not mapped to the objective effect. Use a period-indexed Series and map to costs:

-    power_prices = np.array([0.08, 0.09, 0.10])
+    power_prices = pd.Series([0.08, 0.09, 0.10], index=periods, name='period')  # €/kWh by period
...
-    power_sink = fx.Sink(
-        label='Einspeisung', inputs=[fx.Flow(label='P_el', bus='Strom', effects_per_flow_hour=-1 * power_prices)]
-    )
+    power_sink = fx.Sink(
+        label='Einspeisung',
+        inputs=[fx.Flow(label='P_el', bus='Strom', effects_per_flow_hour={costs.label: -power_prices})],
+    )

This ensures correct effect mapping and coordinate alignment.

Also applies to: 105-107

🤖 Prompt for AI Agents
In examples/04_Scenarios/scenario_example.py around lines 22-23 (and also lines
105-107), passing a raw numpy array for power_prices to effects_per_flow_hour is
ambiguous and not aligned to the model periods; replace the ndarray with a
pandas Series indexed by the scenario/periods (e.g., pd.Series(power_prices,
index=periods)) and then assign that Series to the objective mapping (e.g., map
to 'costs' via effects_per_flow_hour with explicit period-aligned coordinates)
so the effect is explicit and correctly aligned to the model periods.

Comment on lines +198 to +203
# === Treat flixopt warnings as errors (strict mode for our code) ===
# This ensures we catch deprecations, future changes, and user warnings in our own code
"error::DeprecationWarning:flixopt",
"error::FutureWarning:flixopt",
"error::UserWarning:flixopt",

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Allow-list deprecation used by tests to prevent failures.

Tests assert on flixopt deprecations (e.g., change_logging_level) using pytest.warns. Globally promoting flixopt DeprecationWarning to error will break those tests. Add a message-scoped default rule before the blanket errors.

Apply this diff:

 filterwarnings = [
     # === Default behavior: show all warnings ===
     "default",
 
     # === Treat flixopt warnings as errors (strict mode for our code) ===
     # This ensures we catch deprecations, future changes, and user warnings in our own code
+    # Allow specific deprecation expected by tests to remain a warning
+    "default:change_logging_level is deprecated:DeprecationWarning:flixopt",
     "error::DeprecationWarning:flixopt",
     "error::FutureWarning:flixopt",
     "error::UserWarning:flixopt",

Reference: tests/test_config.py expects a DeprecationWarning from flixopt.change_logging_level.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# === Treat flixopt warnings as errors (strict mode for our code) ===
# This ensures we catch deprecations, future changes, and user warnings in our own code
"error::DeprecationWarning:flixopt",
"error::FutureWarning:flixopt",
"error::UserWarning:flixopt",
filterwarnings = [
# === Default behavior: show all warnings ===
"default",
# === Treat flixopt warnings as errors (strict mode for our code) ===
# This ensures we catch deprecations, future changes, and user warnings in our own code
# Allow specific deprecation expected by tests to remain a warning
"default:change_logging_level is deprecated:DeprecationWarning:flixopt",
"error::DeprecationWarning:flixopt",
"error::FutureWarning:flixopt",
"error::UserWarning:flixopt",
]
🤖 Prompt for AI Agents
In pyproject.toml around lines 198-203, the blanket rules escalate all flixopt
DeprecationWarnings to errors which breaks tests that intentionally assert
specific deprecations; insert a message-scoped allow/default rule immediately
before the three blanket "error::...:flixopt" lines that targets the specific
flixopt DeprecationWarning message(s) used by tests (e.g., the
change_logging_level deprecation) so those warnings remain warnings for test
assertions while leaving the remaining flixopt warnings promoted to errors.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (3)
pyproject.toml (1)

195-211: Reorder filters: specific ignore must come before blanket error.

Pytest applies the first matching filter. With error::UserWarning:flixopt before the message-specific ignore, the “network visualization is still experimental” warning becomes an error.

Apply:

 filterwarnings = [
     # === Default behavior: show all warnings ===
     "default",
 
     # === Treat flixopt warnings as errors (strict mode for our code) ===
     # This ensures we catch deprecations, future changes, and user warnings in our own code
+    "ignore:.*network visualization is still experimental.*:UserWarning:flixopt",
     "error::DeprecationWarning:flixopt",
     "error::FutureWarning:flixopt",
     "error::UserWarning:flixopt",
 
     # === Third-party warnings (mirrored from __init__.py) ===
     "ignore::FutureWarning:tsam",
     "ignore:.*minimal value.*exceeds.*:UserWarning:tsam",
     "ignore:Coordinates across variables not equal:UserWarning:linopy",
     "ignore:.*join will change from join='outer' to join='exact'.*:FutureWarning:linopy",
     "ignore:numpy\\.ndarray size changed:RuntimeWarning",
-    "ignore:.*network visualization is still experimental.*:UserWarning:flixopt",
 ]

Optionally broaden the NumPy ABI warning ignore to cover “ufunc size changed” variants:

-    "ignore:numpy\\.ndarray size changed:RuntimeWarning",
+    "ignore:.*size changed.*:RuntimeWarning",

Only if you intentionally want to suppress all such ABI warnings in tests.

examples/03_Calculation_types/example_calculation_types.py (1)

226-230: Clarify plotting intent: stacked vs. grouped bars.

Line 227 specifies style='stacked_bar' but line 230 overrides with .update_layout(barmode='group'), producing grouped bars instead. This inconsistency was previously flagged: if grouped bars are desired, use style='grouped_bar' directly; if stacked bars are intended, remove the barmode='group' override.

CHANGELOG.md (1)

180-180: Remove stray “Until here -->” marker.

Leftover debug marker breaks formatting and tooling. Delete the line.

-Until here -->
🧹 Nitpick comments (13)
pyproject.toml (1)

52-53: Relax numexpr cap; exclude the bad release only.

Current <2.14 blocks future 2.14.x fixes. Prefer excluding 2.14.0 and allowing later 2.x on Py<3.11.

Apply:

-    "numexpr >= 2.8.4, < 2.14; python_version < '3.11'",  # Avoid 2.14.0 on older Python
+    "numexpr >= 2.8.4, != 2.14.0, < 3; python_version < '3.11'",  # Avoid 2.14.0 on older Python; allow future 2.x

Please confirm this still resolves the Py3.10 + NumPy 1.26.4 issue in your CI matrix.

examples/02_Complex/complex_example.py (1)

193-193: LGTM! Network app start includes helpful dependency warning.

The comment clearly informs users about the optional dependency requirement.

If you want more graceful handling, consider wrapping the call:

-flow_system.start_network_app()  # Start the network app. Does only work with extra dependencies installed
+try:
+    flow_system.start_network_app()  # Start the network app
+except ImportError as e:
+    print(f"Network app requires extra dependencies: {e}")
.github/workflows/python-app.yaml (5)

90-92: Surface warnings during tests (avoid hiding deprecations)

Dropping the warnings plugin helps catch deprecations before releasing v3.

-      - name: Run tests
-        run: pytest -v -p no:warnings --numprocesses=auto
+      - name: Run tests
+        run: pytest -v --numprocesses=auto

231-237: Migrate TestPyPI upload to OIDC (Trusted Publishing)

Removes long‑lived tokens and simplifies secrets management.

-      - name: Upload to TestPyPI
-        run: |
-          twine upload --repository-url https://test.pypi.org/legacy/ dist/* --verbose --skip-existing
-        env:
-          TWINE_USERNAME: __token__
-          TWINE_PASSWORD: ${{ secrets.TEST_PYPI_API_TOKEN }}
-          TWINE_NON_INTERACTIVE: "1"
+      - name: Upload to TestPyPI (OIDC)
+        uses: pypa/gh-action-pypi-publish@release/v1
+        with:
+          repository-url: https://test.pypi.org/legacy/
+          packages-dir: dist/
+          skip-existing: true

Note: Requires enabling Trusted Publishing for your TestPyPI project.


317-324: Migrate PyPI upload to OIDC (Trusted Publishing)

Use the official action and remove API tokens.

-      - name: Upload to PyPI
-        run: |
-          twine upload dist/* --verbose --skip-existing
-        env:
-          TWINE_USERNAME: __token__
-          TWINE_PASSWORD: ${{ secrets.PYPI_API_TOKEN }}
-          TWINE_NON_INTERACTIVE: "1"
+      - name: Upload to PyPI (OIDC)
+        uses: pypa/gh-action-pypi-publish@release/v1
+        with:
+          packages-dir: dist/
+          skip-existing: true

Note: Requires enabling Trusted Publishing for your PyPI project.


187-194: Avoid mixing body_path with generate_release_notes

When body_path is provided, auto‑generated notes are redundant. Pick one.

       - name: Create GitHub Release
         uses: softprops/action-gh-release@v2
         with:
           body_path: current_release_notes.md
           draft: false
-          prerelease: ${{ contains(github.ref, 'alpha') || contains(github.ref, 'beta') || contains(github.ref, 'rc') }}
-          generate_release_notes: true
+          prerelease: ${{ contains(github.ref, 'alpha') || contains(github.ref, 'beta') || contains(github.ref, 'rc') }}

33-40: Pin actions to commit SHAs for supply‑chain hardening

Replace version tags (e.g., actions/checkout@v5) with immutable commit SHAs across the workflow.

Also applies to: 41-45, 72-79, 81-85, 100-109, 110-114, 127-135, 136-140, 165-175, 176-180, 209-217, 218-222, 295-303, 304-308, 379-389, 390-394

docs/index.md (2)

13-14: Clarify support status for periods/scenarios.

Text says “Full support …” and “in active development” simultaneously. Reword to avoid contradiction and set accurate expectations.


19-21: Add reference link for “marimo”.

Link the tool for readers unfamiliar with it, e.g., https://marimo.io

CHANGELOG.md (4)

99-103: Consistent naming/casing.

Use consistent casing for “FlowSystem” and avoid snake_case in prose (“flow_system”). Example:

  • “variables and constraints for the FlowSystem.”

74-76: Use code formatting consistently for dataset/function names.

Surround identifiers like effects_per_component with backticks for consistency.

-- `effects_per_component()` dataset
+- `effects_per_component` dataset

Also applies to: 147-154


161-168: Standardize “modeling/modelling”.

Use a single variant across the file (prefer “modeling” to match other entries).


86-92: Add migration pointer for breaking changes.

Consider a short “Migration notes” link/section guiding users to update patterns (Effect sharing, class renames, return type of Calculation.do_modeling()).

I can draft a minimal migration checklist if helpful.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3bc90f4 and e7d2e9d.

⛔ Files ignored due to path filters (1)
  • examples/resources/Zeitreihen2020.csv is excluded by !**/*.csv
📒 Files selected for processing (10)
  • .github/workflows/python-app.yaml (1 hunks)
  • CHANGELOG.md (2 hunks)
  • docs/index.md (1 hunks)
  • examples/02_Complex/complex_example.py (7 hunks)
  • examples/03_Calculation_types/example_calculation_types.py (7 hunks)
  • examples/04_Scenarios/scenario_example.py (1 hunks)
  • examples/05_Two-stage-optimization/two_stage_optimization.py (1 hunks)
  • flixopt/__init__.py (2 hunks)
  • mkdocs.yml (1 hunks)
  • pyproject.toml (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • mkdocs.yml
  • examples/04_Scenarios/scenario_example.py
🧰 Additional context used
🧬 Code graph analysis (3)
examples/02_Complex/complex_example.py (3)
flixopt/config.py (3)
  • CONFIG (61-322)
  • Logging (88-173)
  • apply (208-244)
flixopt/effects.py (1)
  • Effect (32-387)
flixopt/components.py (2)
  • Source (1115-1219)
  • Sink (1223-1342)
examples/03_Calculation_types/example_calculation_types.py (5)
flixopt/config.py (3)
  • CONFIG (61-322)
  • Logging (88-173)
  • apply (208-244)
flixopt/core.py (3)
  • TimeSeriesData (42-150)
  • aggregation_weight (109-110)
  • aggregation_group (105-106)
flixopt/components.py (2)
  • Sink (1223-1342)
  • Source (1115-1219)
flixopt/solvers.py (1)
  • HighsSolver (61-81)
flixopt/plotting.py (1)
  • with_plotly (328-466)
examples/05_Two-stage-optimization/two_stage_optimization.py (3)
flixopt/results.py (3)
  • flow_system (290-306)
  • size (1283-1291)
  • sizes (433-460)
flixopt/flow_system.py (3)
  • FlowSystem (46-980)
  • resample (926-976)
  • sel (852-887)
flixopt/calculation.py (5)
  • FullCalculation (176-266)
  • do_modeling (191-199)
  • do_modeling (311-325)
  • solve (228-266)
  • fix_sizes (201-226)
⏰ 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.11)
  • GitHub Check: test (3.13)
  • GitHub Check: test (3.10)
  • GitHub Check: test (3.12)
🔇 Additional comments (25)
pyproject.toml (1)

189-190: LGTM: cross‑platform pytest marker quoting fixed.

The addopts string now tokenizes on Windows and POSIX.

flixopt/__init__.py (3)

5-6: LGTM! Standard imports for version handling and warning management.

The use of importlib.metadata is the modern Python approach for retrieving package metadata.


8-12: LGTM! Standard version fallback pattern.

The try/except pattern with a development mode fallback is a common and appropriate approach for handling package metadata in various installation scenarios.


69-71: Verify manually: install NumPy and test for “numpy.ndarray size changed” warnings
CI environment lacks NumPy. In your local setup, install NumPy (≥2.x) and any compiled‐extension deps (e.g. pandas, xarray), then rerun imports to see if the RuntimeWarning still occurs. If not, remove the filter; if it does, update the comment to specify which dependency triggers it and link the upstream issue.

examples/05_Two-stage-optimization/two_stage_optimization.py (3)

1-20: LGTM! Well-structured example demonstrating two-stage optimization.

The module docstring clearly explains the downsampling approach and its benefits. The imports and setup are appropriate for this example.

I note that the previous timer and naming issues flagged in earlier reviews have been successfully addressed (start timer reset on line 131, correct 'Dispatch' naming on line 132).


124-136: Two-stage optimization correctly implemented.

The sizing phase uses a downsampled system (resample('4h')) to reduce computational cost, then the dispatch phase uses the full system with sizes fixed from the sizing results. The timer resets and calculation naming are now correct.


150-179: Comparison logic and output are well-structured.

The results comparison correctly:

  • Concatenates solutions from both approaches along a 'mode' dimension
  • Aggregates timing (sizing + dispatch for two-stage)
  • Computes percentage differences
  • Presents a clear comparison of key metrics (durations, costs, component sizes)
examples/02_Complex/complex_example.py (8)

12-14: LGTM! Logging configuration follows the new CONFIG API correctly.

The console logging setup uses the proper v3 API pattern with CONFIG.Logging.console = True followed by CONFIG.apply() to activate the configuration.


46-47: LGTM! Effect sharing parameter correctly implements carbon pricing.

The share_from_temporal={'CO2': 0.2} parameter on the Costs effect establishes a 0.2 €/kg carbon price, which is a standard approach for modeling CO2 emission costs in energy systems. This aligns with v3's redesigned effect sharing mechanism.


63-66: LGTM! InvestParameters correctly use the renamed v3 API.

The parameter renames (effects_of_investment, mandatory, effects_of_investment_per_size) align with the v3 public API changes and are applied correctly.


78-79: LGTM! Time-varying on-off constraints are correctly specified.

The consecutive_on_hours_min array correctly has 9 elements matching the 9 timesteps, demonstrating time-varying operational constraints where minimum consecutive on hours increase to 2 after period 5.


135-136: LGTM! Storage investment parameters correctly use the renamed v3 API.

The piecewise_effects_of_investment parameter name aligns with v3's API changes and properly references the segmented investment effects defined above.


152-159: LGTM! Sink component correctly migrated to v3 list-based API.

The migration from the deprecated sink kwarg to inputs=[fx.Flow(...)] follows the new v3 API pattern shown in the Sink class definition.


165-172: LGTM! Source component correctly migrated to v3 list-based API.

The migration from the deprecated source kwarg to outputs=[fx.Flow(...)] follows the new v3 API pattern shown in the Source class definition.


178-184: LGTM! Feed-in sink correctly migrated to v3 list-based API.

The migration follows the same pattern as the other Sink components, correctly using the negative electricity price for feed-in revenue modeling.

examples/03_Calculation_types/example_calculation_types.py (6)

14-16: LGTM! Logging configuration follows the correct API pattern.

The console logging setup correctly uses fx.CONFIG.Logging.console = True followed by fx.CONFIG.apply() to activate the settings, consistent with the configuration API introduced in v3.


53-55: LGTM! Correct usage of the updated TimeSeriesData aggregation API.

The TimeSeriesData instantiations correctly use the new aggregation_weight and aggregation_group parameters instead of the deprecated agg_weight and agg_group. Line 54 also correctly uses electricity_price instead of the previously incorrect electricity_demand.


116-153: LGTM! Sink and Source definitions updated to the new API.

All Sink and Source component instantiations correctly use the updated API with inputs=[...] and outputs=[...] list parameters instead of the deprecated sink= and source= parameters. Flow definitions are properly wrapped in lists as expected by the new signature.


176-176: LGTM! Reasonable MIP gap relaxation for improved performance.

The solver configuration has been updated from an exact solution (mip_gap=0) to a 0.01% relative optimality gap (0.01/100 = 0.0001). This small relaxation can significantly reduce solve time while maintaining solution quality within 0.01% of optimal, applied consistently across Full, Segmented, and Aggregated calculations with a 60-second time limit.

Also applies to: 181-181, 190-190


206-206: LGTM! Appropriate plotting style for time-series comparison.

The use of style='line' for visualizing charge state, flow rate, and temporal cost comparisons is appropriate for these time-series data, providing clear stepped line plots that facilitate comparison across calculation types.

Also applies to: 213-213, 220-220


233-233: Valid durations attribute: calc.durations is defined in the Calculation base class (flixopt/calculation.py:93) and correctly populated for all calculation types.

.github/workflows/python-app.yaml (2)

8-9: PR trigger patterns: verify intended coverage

Globs look fine and cover dev and feature branches. Do you also want PRs into release/* or hotfix/* to run? If yes, add 'release/' and/or 'hotfix/'.


181-186: Helper scripts and extras verified scripts/extract_release_notes.py and scripts/extract_changelog.py are present; ‘dev’ and ‘docs’ optional-dependencies include pytest, pytest-xdist, mkdocs, and mike.

docs/index.md (2)

84-87: Image path is valid. ./images/architecture_flixOpt.png exists in docs/images and will be included in the docs build.


61-63: Example’s use of boiler.model.add_constraints is already correct for v3’s Submodel APIboiler.model returns a Submodel, so its add_constraints method properly scopes new constraints. No update needed.

Comment on lines +22 to 23
- **Recipe collection**: Community-driven library of common modeling patterns, data manipulation techniques, and optimization strategies (see [Recipes](user-guide/recipes/index.md) - help wanted!)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Validate internal doc links.

Check that these targets exist:

  • user-guide/recipes/index.md
  • getting-started.md

Also applies to: 95-96


🏁 Script executed:

#!/bin/bash
# From repo root; look under docs/
fd -a -HI 'docs' -t d >/dev/null || echo "No docs/ dir found"
fd -a -HI 'user-guide/recipes/index.md' 'docs' -x echo "{}"
fd -a -HI 'getting-started.md' 'docs' -x echo "{}"

Length of output: 42


🏁 Script executed:

#!/bin/bash
# From repo root, search under docs/
fd -HI -t d user-guide docs
fd -HI -t d recipes docs
fd -HI -t f '*recipe*' docs

Length of output: 304


🏁 Script executed:

#!/bin/bash
# From repo root; search for any “recipe” entries under docs/user-guide
fd -HI -t d --glob '*recipe*' docs/user-guide
# List all index.md files under docs/user-guide
fd -HI -t f index.md docs/user-guide

Length of output: 136


Ensure internal documentation links point to valid targets

  • user-guide/recipes/index.md is missing; update to user-guide/mathematical-notation/modeling-patterns/index.md
  • getting-started.md is present at docs/getting-started.md and requires no change
🤖 Prompt for AI Agents
In docs/index.md around lines 22 to 23, the internal link points to a missing
target user-guide/recipes/index.md; update that link to
user-guide/mathematical-notation/modeling-patterns/index.md so the markdown
reference resolves correctly (no change needed for getting-started.md).

Comment on lines 51 to 54
warnings.filterwarnings('ignore', category=FutureWarning, module='tsam')
# - UserWarning: Informational message about minimal value constraints
warnings.filterwarnings('ignore', category=UserWarning, message='.*minimal value.*exceeds.*', module='tsam')
# TODO: Might be able to fix it in flixopt?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Narrow the tsam FutureWarning suppression and resolve the TODO.

The current filter on line 51 suppresses all FutureWarnings from tsam without a message pattern, which is too broad and may hide important API change warnings that users should be aware of.

The TODO comment suggests uncertainty about whether this should be fixed within flixopt itself. Before suppressing these warnings globally, investigate whether the underlying issue can be addressed in flixopt's usage of tsam.

Consider one of these approaches:

  1. Narrow the filter to specific known warnings:
-warnings.filterwarnings('ignore', category=FutureWarning, module='tsam')
+# Specify the exact FutureWarning message pattern(s) that should be suppressed
+warnings.filterwarnings('ignore', category=FutureWarning, message='<specific warning pattern>', module='tsam')
  1. Fix in flixopt if the warning indicates incorrect usage of tsam APIs (as the TODO suggests)

  2. Remove the suppression and let users see these warnings if they're actionable

Since this is a major version release (v3.0.0), it's an appropriate time to address this TODO and avoid carrying forward overly broad suppressions.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
warnings.filterwarnings('ignore', category=FutureWarning, module='tsam')
# - UserWarning: Informational message about minimal value constraints
warnings.filterwarnings('ignore', category=UserWarning, message='.*minimal value.*exceeds.*', module='tsam')
# TODO: Might be able to fix it in flixopt?
# Narrow the tsam FutureWarning suppression to specific known warnings only
# Specify the exact FutureWarning message pattern(s) that should be suppressed
warnings.filterwarnings(
'ignore',
category=FutureWarning,
message='<specific warning pattern>',
module='tsam'
)
# - UserWarning: Informational message about minimal value constraints
warnings.filterwarnings(
'ignore',
category=UserWarning,
message='.*minimal value.*exceeds.*',
module='tsam'
)
# TODO: Might be able to fix it in flixopt?
🤖 Prompt for AI Agents
In flixopt/__init__.py around lines 51-54 the code suppresses all FutureWarning
from the 'tsam' module which is too broad; investigate test runs to reproduce
the specific FutureWarning message(s) emitted by tsam, then either (A) update
the warnings.filterwarnings call to include a narrow message regex matching only
the known harmless deprecation(s) (and keep module='tsam'), or (B) if the
warning indicates incorrect usage, change the flixopt code that triggers it so
the deprecation is not emitted and remove the suppression; finally remove the
TODO comment once you either narrow the filter or fix the usage and add a brief
comment explaining the chosen approach.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (6)
CHANGELOG.md (1)

69-71: Remove stray “Until here -->” marker

This leftover marker breaks the changelog format; drop it so only the separator remains.

----
-Until here -->
 
## [3.0.0] - 2025-10-13
flixopt/elements.py (4)

586-593: Use non-deprecated self.investment property.

Line 589 references self._investment.size, but the property at lines 695-697 marks _investment as deprecated. Use the current self.investment property for consistency.

Apply this diff:

             BoundingPatterns.scaled_bounds_with_state(
                 model=self,
                 variable=self.flow_rate,
-                scaling_variable=self._investment.size,
+                scaling_variable=self.investment.size,
                 relative_bounds=self.relative_flow_rate_bounds,
                 scaling_bounds=(self.element.size.minimum_or_fixed_size, self.element.size.maximum_or_fixed_size),
                 variable_state=self.on_off.on,
             )

203-209: Fix ambiguous truth-value check for xarray DataArray.

np.all(np.equal(...)) on xarray returns a 0-D DataArray, causing ambiguous truth-value when used in if. Convert to Python bool.

Apply this diff:

         if self.excess_penalty_per_flow_hour is not None:
-            zero_penalty = np.all(np.equal(self.excess_penalty_per_flow_hour, 0))
+            zero_penalty = bool(np.all(np.asarray(self.excess_penalty_per_flow_hour) == 0))
             if zero_penalty:
                 logger.warning(
                     f'In Bus {self.label_full}, the excess_penalty_per_flow_hour is 0. Use "None" or a value > 0.'
                 )

449-456: Fix ambiguous boolean and reorder conditions for efficiency.

After transform_data, self.size (when not InvestParameters) is xarray DataArray. np.any(self.size == ...) returns 0-D DataArray, causing ambiguous truth-value. Also, reorder conditions to short-circuit.

Apply this diff:

         if not isinstance(self.size, InvestParameters) and (
-            np.any(self.size == CONFIG.Modeling.big) and self.fixed_relative_profile is not None
+            self.fixed_relative_profile is not None and bool(np.any(np.asarray(self.size) == CONFIG.Modeling.big))
         ):  # Default Size --> Most likely by accident
             logger.warning(
                 f'Flow "{self.label_full}" has no size assigned, but a "fixed_relative_profile". '
                 f'The default size is {CONFIG.Modeling.big}. As "flow_rate = size * fixed_relative_profile", '
                 f'the resulting flow_rate will be very high. To fix this, assign a size to the Flow {self}.'
             )

446-447: Fix ambiguous truth-value in relative bounds check.

After transform_data, relative_minimum and relative_maximum are xarray DataArrays. The comparison returns DataArray, and .any() returns 0-D DataArray, causing ambiguous truth-value in if.

Apply this diff:

-        if (self.relative_minimum > self.relative_maximum).any():
+        if bool((self.relative_minimum > self.relative_maximum).any()):
             raise PlausibilityError(self.label_full + ': Take care, that relative_minimum <= relative_maximum!')
tests/conftest.py (1)

671-672: Derive profile lengths from flow_system.timesteps for robustness.

Hardcoded lengths (10) will break if coords_config is updated with different timestep counts. Derive the length from the constructed flow_system instead.

Apply this diff:

+    n = len(flow_system.timesteps)
-    thermal_load = LoadProfiles.random_thermal(10)
-    p_el = LoadProfiles.random_electrical(10)
+    thermal_load = LoadProfiles.random_thermal(n)
+    p_el = LoadProfiles.random_electrical(n)
🧹 Nitpick comments (2)
CHANGELOG.md (2)

81-83: Normalize nested list indentation to 2 spaces (MD007)

Several nested bullets are indented with 4 spaces; markdownlint expects 2. Adjust for consistency and tooling.

Representative fixes:

-    - Control variable independence across scenarios via `scenario_independent_sizes` and `scenario_independent_flow_rates` parameters
-    - By default, investment sizes are shared across scenarios while flow rates vary per scenario
+  - Control variable independence across scenarios via `scenario_independent_sizes` and `scenario_independent_flow_rates` parameters
+  - By default, investment sizes are shared across scenarios while flow rates vary per scenario
-    - Model: The main Model (linopy.Model) that is used to create and store the variables and constraints for the flow_system.
-    - Submodel: The base class for all submodels. Each is a subset of the Model, for simpler access and clearer code.
+  - Model: The main Model (linopy.Model) that is used to create and store the variables and constraints for the flow_system.
+  - Submodel: The base class for all submodels. Each is a subset of the Model, for simpler access and clearer code.
-    - `fix_effects` → `effects_of_investment`
-    - `specific_effects` → `effects_of_investment_per_size`
-    - `divest_effects` → `effects_of_retirement`
-    - `piecewise_effects` → `piecewise_effects_of_investment`
+  - `fix_effects` → `effects_of_investment`
+  - `specific_effects` → `effects_of_investment_per_size`
+  - `divest_effects` → `effects_of_retirement`
+  - `piecewise_effects` → `piecewise_effects_of_investment`
-    - `minimum_investment` → `minimum_periodic`
-    - `maximum_investment` → `maximum_periodic`
-    - `minimum_operation` → `minimum_temporal`
-    - `maximum_operation` → `maximum_temporal`
-    - `minimum_operation_per_hour` → `minimum_per_hour`
-    - `maximum_operation_per_hour` → `maximum_per_hour`
+  - `minimum_investment` → `minimum_periodic`
+  - `maximum_investment` → `maximum_periodic`
+  - `minimum_operation` → `minimum_temporal`
+  - `maximum_operation` → `maximum_temporal`
+  - `minimum_operation_per_hour` → `minimum_per_hour`
+  - `maximum_operation_per_hour` → `maximum_per_hour`
-    - `fit_to_model_coords()` method for data alignment
-    - `fit_effects_to_model_coords()` method for effect data processing
-    - `connect_and_transform()` method replacing several operations
+  - `fit_to_model_coords()` method for data alignment
+  - `fit_effects_to_model_coords()` method for effect data processing
+  - `connect_and_transform()` method replacing several operations
-    - Updated deprecated code patterns in tests and examples (e.g., `sink`/`source` → `inputs`/`outputs`, `'H'` → `'h'` frequency)
-    - Refactored plotting logic to handle test environments explicitly with non-interactive backends
-    - Added comprehensive warning filters in `__init__.py` and `pyproject.toml` to suppress third-party library warnings
-    - Improved test fixtures with proper figure cleanup to prevent memory leaks
-    - Enhanced backend detection and handling in `plotting.py` for both Matplotlib and Plotly
-    - Always run dependent tests in order
+  - Updated deprecated code patterns in tests and examples (e.g., `sink`/`source` → `inputs`/`outputs`, `'H'` → `'h'` frequency)
+  - Refactored plotting logic to handle test environments explicitly with non-interactive backends
+  - Added comprehensive warning filters in `__init__.py` and `pyproject.toml` to suppress third-party library warnings
+  - Improved test fixtures with proper figure cleanup to prevent memory leaks
+  - Enhanced backend detection and handling in `plotting.py` for both Matplotlib and Plotly
+  - Always run dependent tests in order

From static analysis hints (MD007).

Also applies to: 127-129, 141-152, 153-158, 196-198, 200-205


221-226: Move 2.2.0 breaking bullets into a dedicated “💥 Breaking Changes” section

Release-note tooling keys on the 💥 header. Keep “Changed” for non-breaking items and list the two breaking bullets under a new section.

 ## [2.2.0] - 2025-10-11
 ### ✨ Added
 ...
 ### ♻️ Changed
 - Logging and Configuration management changed
-- **Breaking**: Console logging is now disabled by default (`CONFIG.Logging.console = False`). Enable it explicitly in your scripts with `CONFIG.Logging.console = True` and `CONFIG.apply()`
-- **Breaking**: File logging is now disabled by default (`CONFIG.Logging.file = None`). Set a file path to enable file logging
 - Improved default logging colors: DEBUG is now gray (`\033[90m`) for de-emphasized messages, INFO uses terminal default color (`\033[0m`) for clean output
+
+### 💥 Breaking Changes
+- Console logging is now disabled by default (`CONFIG.Logging.console = False`). Enable it explicitly with `CONFIG.Logging.console = True` and `CONFIG.apply()`
+- File logging is now disabled by default (`CONFIG.Logging.file = None`). Set a file path to enable file logging

As per prior tooling guidance in earlier reviews.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e7d2e9d and 8cb3866.

📒 Files selected for processing (3)
  • CHANGELOG.md (3 hunks)
  • flixopt/elements.py (11 hunks)
  • tests/conftest.py (11 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
flixopt/elements.py (7)
flixopt/config.py (2)
  • CONFIG (61-322)
  • Modeling (175-186)
flixopt/features.py (9)
  • InvestmentModel (24-144)
  • OnOffModel (147-320)
  • size (135-137)
  • _do_modeling (50-53)
  • _do_modeling (176-246)
  • _do_modeling (340-362)
  • _do_modeling (396-450)
  • _do_modeling (479-510)
  • _do_modeling (545-568)
flixopt/interface.py (10)
  • InvestParameters (672-1057)
  • OnOffParameters (1061-1326)
  • transform_data (81-84)
  • transform_data (227-229)
  • transform_data (453-455)
  • transform_data (665-668)
  • transform_data (930-984)
  • transform_data (1270-1297)
  • minimum_or_fixed_size (1041-1042)
  • maximum_or_fixed_size (1045-1046)
flixopt/modeling.py (9)
  • BoundingPatterns (390-759)
  • ModelingPrimitives (197-387)
  • ModelingUtilitiesAbstract (14-116)
  • expression_tracking_variable (201-237)
  • bounds_with_state (428-471)
  • scaled_bounds (474-514)
  • scaled_bounds_with_state (517-571)
  • to_binary (18-52)
  • mutual_exclusivity_constraint (345-387)
flixopt/structure.py (16)
  • Element (856-900)
  • ElementModel (1149-1170)
  • FlowSystemModel (82-253)
  • create_model (874-875)
  • _plausibility_checks (869-872)
  • transform_data (274-284)
  • label_full (878-879)
  • label_full (1017-1018)
  • _do_modeling (1077-1079)
  • get_coords (189-218)
  • get_coords (986-991)
  • hours_per_step (182-183)
  • hours_per_step (1074-1075)
  • add_submodels (71-79)
  • results_structure (1165-1170)
  • values (1137-1138)
flixopt/effects.py (10)
  • effects (579-580)
  • create_model (380-383)
  • create_model (464-467)
  • _plausibility_checks (385-387)
  • _plausibility_checks (521-539)
  • transform_data (340-378)
  • _do_modeling (396-433)
  • _do_modeling (673-686)
  • add_share_to_effects (646-666)
  • add_share_to_penalty (668-671)
flixopt/flow_system.py (5)
  • create_model (466-478)
  • FlowSystem (46-980)
  • fit_to_model_coords (362-400)
  • fit_effects_to_model_coords (402-425)
  • coords (769-775)
tests/conftest.py (2)
flixopt/structure.py (3)
  • FlowSystemModel (82-253)
  • values (1137-1138)
  • weights (221-231)
flixopt/core.py (3)
  • TimeSeriesData (42-150)
  • aggregation_weight (109-110)
  • aggregation_group (105-106)
🪛 markdownlint-cli2 (0.18.1)
CHANGELOG.md

81-81: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


82-82: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


127-127: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


128-128: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


141-141: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


142-142: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


143-143: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


144-144: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


146-146: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


147-147: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


148-148: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


149-149: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


150-150: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


151-151: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


153-153: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


154-154: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


155-155: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


156-156: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


157-157: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


162-162: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


163-163: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


196-196: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


197-197: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


198-198: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


200-200: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


201-201: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


202-202: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


203-203: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


204-204: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


205-205: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)

⏰ 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.12)
  • GitHub Check: test (3.10)
  • GitHub Check: test (3.11)
🔇 Additional comments (8)
tests/conftest.py (4)

80-391: LGTM! Well-structured hierarchical test library.

The new hierarchical library (Buses, Effects, Converters, Storage, LoadProfiles, Sinks, Sources) centralizes reusable test components and reduces duplication across fixtures. The design properly parameterizes lengths and provides clear, discoverable test building blocks.


398-639: LGTM! Fixtures consistently leverage hierarchical library.

The updated fixtures properly use the new hierarchical library classes and derive lengths from timesteps where needed. The integration maintains backward compatibility while reducing code duplication.


792-811: LGTM! Clear and helpful set comparison utility.

The assert_sets_equal function provides informative error messages distinguishing extra vs. missing elements, improving test debugging.


818-850: LGTM! Proper test environment configuration.

The new plotting fixtures correctly configure non-interactive backends (matplotlib 'Agg', plotly 'json') and clean up figures, preventing GUI windows and memory leaks in test/CI environments.

flixopt/elements.py (4)

15-19: LGTM! Imports correctly updated.

The imports properly reference:

  • ModelingPrimitives from .modeling (line 18)
  • InvestmentModel, OnOffModel from .features (line 16)
  • FlowSystemModel from .structure (line 19)

This aligns with the new submodel architecture.


493-720: LGTM! Well-structured FlowModel refactoring.

The FlowModel refactoring properly implements the submodel architecture with:

  • Clean separation of concerns (_do_modeling, _create_on_off_model, _create_investment_model, _constraint_flow_rate)
  • Proper variable registration via add_variables with short_name
  • Feature composition via add_submodels
  • Clear property access patterns for flow_rate, total_flow_hours, on_off, investment
  • Correct use of BoundingPatterns and ModelingPrimitives helpers

The design maintains maintainability and extensibility.


723-767: LGTM! BusModel properly integrates with submodel architecture.

The BusModel refactoring correctly:

  • Accesses flow rates via flow.submodel.flow_rate (lines 736-737)
  • Creates bus balance constraints using summed flows
  • Handles excess variables and penalties when configured
  • Provides comprehensive results_structure with inputs, outputs, flows

The integration with the FlowModel submodels is clean and correct.


770-852: LGTM! ComponentModel orchestration well-implemented.

The ComponentModel refactoring properly:

  • Creates flow submodels via flow.create_model(self._model) (line 792)
  • Links component-level on with flow-level ons using appropriate constraints (lines 795-804)
  • Creates component-level OnOffModel with correct parameters (lines 806-816)
  • Handles mutual exclusivity constraints (lines 820-824)
  • Aggregates previous_states from flow submodels with proper padding (lines 835-852)

The orchestration maintains clear separation between component and flow concerns.

Comment on lines +464 to 469
if np.any(self.relative_minimum > 0) and self.on_off_parameters is None:
logger.warning(
f'Flow {self.label} has a relative_minimum of {self.relative_minimum.active_data} and no on_off_parameters. '
f'Flow {self.label_full} has a relative_minimum of {self.relative_minimum} and no on_off_parameters. '
f'This prevents the flow_rate from switching off (flow_rate = 0). '
f'Consider using on_off_parameters to allow the flow to be switched on and off.'
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fix ambiguous boolean in relative_minimum check.

After transform_data, relative_minimum is xarray DataArray. np.any(self.relative_minimum > 0) returns 0-D DataArray, causing ambiguous truth-value in if.

Apply this diff:

-        if np.any(self.relative_minimum > 0) and self.on_off_parameters is None:
+        if bool(np.any(self.relative_minimum > 0)) and self.on_off_parameters is None:
             logger.warning(
                 f'Flow {self.label_full} has a relative_minimum of {self.relative_minimum} and no on_off_parameters. '
                 f'This prevents the flow_rate from switching off (flow_rate = 0). '
                 f'Consider using on_off_parameters to allow the flow to be switched on and off.'
             )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if np.any(self.relative_minimum > 0) and self.on_off_parameters is None:
logger.warning(
f'Flow {self.label} has a relative_minimum of {self.relative_minimum.active_data} and no on_off_parameters. '
f'Flow {self.label_full} has a relative_minimum of {self.relative_minimum} and no on_off_parameters. '
f'This prevents the flow_rate from switching off (flow_rate = 0). '
f'Consider using on_off_parameters to allow the flow to be switched on and off.'
)
if bool(np.any(self.relative_minimum > 0)) and self.on_off_parameters is None:
logger.warning(
f'Flow {self.label_full} has a relative_minimum of {self.relative_minimum} and no on_off_parameters. '
f'This prevents the flow_rate from switching off (flow_rate = 0). '
f'Consider using on_off_parameters to allow the flow to be switched on and off.'
)
🤖 Prompt for AI Agents
In flixopt/elements.py around lines 464-469, the condition uses
np.any(self.relative_minimum > 0) which can be a 0-D xarray DataArray and cause
an ambiguous truth-value error; replace that np.any(...) call with an explicit
boolean check that works for xarray/numpy, e.g. use (self.relative_minimum >
0).any().item() or self.relative_minimum.values.any() so the if becomes: if
(self.relative_minimum > 0).any().item() and self.on_off_parameters is None: and
keep the existing warning block unchanged.

import os
from collections.abc import Iterable

import linopy.testing
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Add missing import linopy to resolve NameError in type annotations.

Functions assert_conequal (line 736) and assert_var_equal (line 755) use linopy.Constraint and linopy.Variable in their signatures. Without import linopy or from __future__ import annotations, these annotations are evaluated at import time and will raise NameError.

Apply this diff:

 import linopy.testing
+import linopy
 import numpy as np
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import linopy.testing
import linopy.testing
import linopy
import numpy as np
🤖 Prompt for AI Agents
In tests/conftest.py around line 10, add an import for the linopy module (e.g.,
import linopy) near the top of the file so that type annotations like
linopy.Constraint and linopy.Variable used later (lines ~736 and ~755) are
available at import time; ensure the import is placed before any code that
declares or evaluates those annotations (alternatively, you could add from
__future__ import annotations at the top, but the simplest fix is to add import
linopy).

* Typos oin CHANGELOG.md

* Add error handling in exmaple

* Surface warnings during tests (avoid hiding deprecations)

* Add missing docs file

* Imrpve Releasnotes of v2.2.0

* Improve docs

* Remove some filterwarnings from tsam
@FBumann FBumann merged commit b4dde06 into main Oct 13, 2025
7 of 8 checks passed
This was referenced Nov 21, 2025
@coderabbitai coderabbitai bot mentioned this pull request Dec 18, 2025
9 tasks
@coderabbitai coderabbitai bot mentioned this pull request Jan 17, 2026
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants