Skip to content

Conversation

@FBumann
Copy link
Member

@FBumann FBumann commented Oct 16, 2025

Description

Update the Migration guide and improve the release notes for missed changes

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

  • Breaking Changes

    • Effect system redesigned with new terminology (operation → temporal, invest → periodic); parameters, result keys, and label-based references updated. FlowSystem is now copied per calculation; components/effects must be referenced by labels. Calculation workflow and plotting parameter signatures changed; logging defaults now disabled.
  • Documentation

    • Migration guide rewritten with mapping tables, examples, and guidance for new APIs, FlowSystem usage, and label-based patterns.
  • Chores

    • Markdown configured to support collapsible details blocks.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 16, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Documentation and changelog updated for a v3.0.0 API redesign: effect-sharing terminology and parameter renames, FlowSystem copy-per-Calculation semantics, label-based Bus/Effect references, Calculation.do_modeling() return change, FlowSystem I/O/selection APIs documented, and mkdocs config extended with pymdownx.details.

Changes

Cohort / File(s) Summary
Changelog
CHANGELOG.md
Documents API/behavior changes: effect-sharing redesign (operation→temporal, invest→periodic), specific_share_to_other_effects_* → share_from_temporal/share_from_periodic, FlowSystem per-Calculation copy semantics, Bus/Effect label-only usage, logging defaults, class renames (SystemModelFlowSystemModel, ModelSubmodel), do_modeling() return semantics, result/parameter renames (e.g., is_investedinvested, final-state parameter renames).
Migration Guide
docs/user-guide/migration-guide-v3.md
Rewrites migration guide for v2.x → v3.0.0: maps old→new terminology and public API names; shows label-based examples (buses/effects); documents FlowSystem methods (to_netcdf, from_netcdf, sel, isel, resample, copy); updates Calculation.do_modeling() workflow and access via .model; provides migration checklist and deprecated-parameter mappings.
Docs config
mkdocs.yml
Adds pymdownx.details to Markdown extensions.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Config as Config (labels)
    participant Calculation
    participant FlowSystem
    participant Results

    rect rgba(0,128,96,0.07)
    User->>Config: define buses/effects using string labels
    end

    rect rgba(0,96,192,0.06)
    User->>Calculation: call Calculation.do_modeling(config)
    Note right of Calculation: do_modeling() returns Calculation\naccess model via Calculation.model
    Calculation->>FlowSystem: create per-Calculation FlowSystem (copy)
    FlowSystem-->>Calculation: attached to Calculation.model
    end

    rect rgba(192,160,0,0.06)
    Calculation->>Results: store results (results.flow_system available)
    User->>Results: call results.flow_system.sel(...)/to_netcdf()/from_netcdf()
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

v3.0.0

Poem

🐰
I hopped from objects to labels bright,
Renamed my hops from day to night.
FlowSystems copied, results set free,
I munch the changelog, nibble v3. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The pull request description uses the required template structure but leaves all change type and testing checkboxes unchecked and retains placeholder text in the related issues section. It omits a concrete selection for the Type of Change and a valid issue number, making key template fields incomplete. This lack of specific information prevents full verification against the repository's description requirements. Please select the appropriate change type (e.g., Documentation update) and provide a real issue number or remove the related issues section if none applies. Mark the testing checkboxes as applicable or indicate that documentation-only changes do not require tests. Filling these fields will ensure the description fully conforms to the repository template.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly and accurately captures the two primary documentation updates made in this pull request. It focuses on the migration guide and changelog changes without unnecessary noise or file listings. The phrasing concisely informs a teammate scanning the history about the main content of the update.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

📜 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 e61aa02 and 7de0ae6.

📒 Files selected for processing (1)
  • CHANGELOG.md (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: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8294a64 and 9f50ca2.

📒 Files selected for processing (2)
  • CHANGELOG.md (1 hunks)
  • docs/user-guide/migration-guide-v3.md (2 hunks)
🧰 Additional context used
🪛 LanguageTool
CHANGELOG.md

[grammar] ~144-~144: There might be a mistake here.
Context: ...ct system redesigned** (no deprecation): - Terminology changes: Effect domains re...

(QB_NEW_EN)


[grammar] ~145-~145: There might be a mistake here.
Context: ...temporal, invest/investmentperiodic - **Sharing system**: The oldspecific_shar...

(QB_NEW_EN)


[grammar] ~146-~146: There might be a mistake here.
Context: ...eriodic` syntax (see 🔥 Removed section) - FlowSystem independence: FlowSystems c...

(QB_NEW_EN)


[grammar] ~155-~155: There might be a mistake here.
Context: ...ng:** - Renamed class SystemModel to FlowSystemModel - Renamed class Model to Submodel - Re...

(QB_NEW_EN)


[grammar] ~156-~156: There might be a mistake here.
Context: ...SystemModel- Renamed classModeltoSubmodel- Renamedmode` parameter in plotting met...

(QB_NEW_EN)


[grammar] ~162-~162: There might be a mistake here.
Context: ...variable: is_investedinvested in InvestmentModel - Switch tracking variables in `OnOffModel...

(QB_NEW_EN)

docs/user-guide/migration-guide-v3.md

[grammar] ~25-~25: There might be a mistake here.
Context: ... | |-----------------|-------------------|-...

(QB_NEW_EN)


[grammar] ~26-~26: There might be a mistake here.
Context: ...---------------------------------------| | operation | temporal | Time-varyin...

(QB_NEW_EN)


[grammar] ~27-~27: There might be a mistake here.
Context: ...tional costs, occuring over time) | | invest / investment | periodic |...

(QB_NEW_EN)


[grammar] ~54-~54: There might be a mistake here.
Context: ...m periodic effects ``` Migration: 1. Move share definitions to the receiving ...

(QB_NEW_EN)


[grammar] ~59-~59: There might be a mistake here.
Context: ...Update terminology throughout your code: - Replace "operation" with "temporal" in e...

(QB_NEW_EN)


[grammar] ~218-~218: There might be a mistake here.
Context: ...Plotting:** mode parameter renamed to style - Class names: SystemModel → `FlowSyst...

(QB_NEW_EN)


[grammar] ~219-~219: There might be a mistake here.
Context: ...emModelFlowSystemModel, ModelSubmodel` - Logging: Disabled by default (enable w...

(QB_NEW_EN)


[grammar] ~228-~228: There might be a mistake here.
Context: ...rk) ### InvestParameters | Old | New | |-----|-----| | fix_effects | `effects...

(QB_NEW_EN)


[grammar] ~229-~229: There might be a mistake here.
Context: ...tParameters | Old | New | |-----|-----| | fix_effects | `effects_of_investment...

(QB_NEW_EN)


[grammar] ~230-~230: There might be a mistake here.
Context: ...fix_effects|effects_of_investment| |specific_effects|effects_of_inves...

(QB_NEW_EN)


[grammar] ~231-~231: There might be a mistake here.
Context: ...ts|effects_of_investment_per_size| |divest_effects|effects_of_retirem...

(QB_NEW_EN)


[grammar] ~232-~232: There might be a mistake here.
Context: ...est_effects|effects_of_retirement| |piecewise_effects|piecewise_effec...

(QB_NEW_EN)


[grammar] ~237-~237: There might be a mistake here.
Context: ...investment| ### Effect | Old | New | |-----|-----| |minimum_investment|...

(QB_NEW_EN)


[grammar] ~238-~238: There might be a mistake here.
Context: ... ### Effect | Old | New | |-----|-----| | minimum_investment | `minimum_period...

(QB_NEW_EN)


[grammar] ~239-~239: There might be a mistake here.
Context: ...nimum_investment|minimum_periodic| |maximum_investment|maximum_period...

(QB_NEW_EN)


[grammar] ~240-~240: There might be a mistake here.
Context: ...ximum_investment|maximum_periodic| |minimum_operation|minimum_tempora...

(QB_NEW_EN)


[grammar] ~241-~241: There might be a mistake here.
Context: ...inimum_operation|minimum_temporal| |maximum_operation|maximum_tempora...

(QB_NEW_EN)


[grammar] ~242-~242: There might be a mistake here.
Context: ...aximum_operation|maximum_temporal| |minimum_operation_per_hour|minimu...

(QB_NEW_EN)


[grammar] ~243-~243: There might be a mistake here.
Context: ...eration_per_hour|minimum_per_hour| |maximum_operation_per_hour|maximu...

(QB_NEW_EN)


[grammar] ~248-~248: There might be a mistake here.
Context: ...our| ### SourceAndSink | Old | New | |-----|-----| |source|outputs` | |...

(QB_NEW_EN)


[grammar] ~249-~249: There might be a mistake here.
Context: ...urceAndSink | Old | New | |-----|-----| | source | outputs | | sink | `inp...

(QB_NEW_EN)


[grammar] ~250-~250: There might be a mistake here.
Context: ...| |-----|-----| | source | outputs | | sink | inputs | | `prevent_simulta...

(QB_NEW_EN)


[grammar] ~251-~251: There might be a mistake here.
Context: ...rce|outputs| |sink|inputs| |prevent_simultaneous_sink_and_source`...

(QB_NEW_EN)


[grammar] ~256-~256: There might be a mistake here.
Context: ...es| ### TimeSeriesData | Old | New | |-----|-----| |agg_group|aggregati...

(QB_NEW_EN)


[grammar] ~257-~257: There might be a mistake here.
Context: ...eSeriesData | Old | New | |-----|-----| | agg_group | aggregation_group | | ...

(QB_NEW_EN)


[grammar] ~258-~258: There might be a mistake here.
Context: ...-| | agg_group | aggregation_group | | agg_weight | aggregation_weight | ...

(QB_NEW_EN)


[grammar] ~333-~333: There might be a mistake here.
Context: ...*"Effect share parameters not working"** → Move shares to receiving effect using ...

(QB_NEW_EN)


[grammar] ~336-~336: There might be a mistake here.
Context: ...age charge state has wrong dimensions"** → Remove extra timestep; use `relative_m...

(QB_NEW_EN)


[grammar] ~339-~339: There might be a mistake here.
Context: ... **"KeyError when accessing results"** → Update variable names: -is_investe...

(QB_NEW_EN)


[grammar] ~348-~348: There might be a mistake here.
Context: ...talEffect **"No logging output"** → Enable explicitly:fx.CONFIG.Logging....

(QB_NEW_EN)


[grammar] ~355-~355: There might be a mistake here.
Context: ...--- ## Migration Checklist Critical: - [ ] Update flixopt: `pip install --upgra...

(QB_NEW_EN)


[grammar] ~356-~356: There might be a mistake here.
Context: ...st Critical: - [ ] Update flixopt: pip install --upgrade flixopt - [ ] Update effect sharing syntax - [ ] U...

(QB_NEW_EN)


[grammar] ~357-~357: There might be a mistake here.
Context: ...xopt` - [ ] Update effect sharing syntax - [ ] Update result variable names - [ ] R...

(QB_NEW_EN)


[grammar] ~358-~358: There might be a mistake here.
Context: ...yntax - [ ] Update result variable names - [ ] Replace object assignments with stri...

(QB_NEW_EN)


[grammar] ~359-~359: There might be a mistake here.
Context: ...ce object assignments with string labels - [ ] Fix storage charge state arrays - [ ...

(QB_NEW_EN)


[grammar] ~360-~360: There might be a mistake here.
Context: ...ls - [ ] Fix storage charge state arrays - [ ] Update do_modeling() usage if need...

(QB_NEW_EN)


[grammar] ~361-~361: There might be a mistake here.
Context: ...] Update do_modeling() usage if needed - [ ] Rename plotting modestyle - [...

(QB_NEW_EN)


[grammar] ~362-~362: There might be a mistake here.
Context: ...f needed - [ ] Rename plotting modestyle - [ ] Enable logging if needed **Recommen...

(QB_NEW_EN)


[grammar] ~365-~365: There might be a mistake here.
Context: ...Enable logging if needed Recommended: - [ ] Update deprecated parameter names - ...

(QB_NEW_EN)


[grammar] ~369-~369: There might be a mistake here.
Context: ...oughly and validate results Optional: - [ ] Explore new features (periods, scena...

(QB_NEW_EN)


[grammar] ~372-~372: There might be a mistake here.
Context: ...rage) Welcome to flixopt v3.0.0! 🎉 --- ## Resources - Docs: https://flixopt.g...

(QB_NEW_EN)

🪛 markdownlint-cli2 (0.18.1)
docs/user-guide/migration-guide-v3.md

36-36: Code block style
Expected: fenced; Actual: indented

(MD046, code-block-style)


46-46: Code block style
Expected: fenced; Actual: indented

(MD046, code-block-style)


86-86: Code block style
Expected: fenced; Actual: indented

(MD046, code-block-style)


104-104: Code block style
Expected: fenced; Actual: indented

(MD046, code-block-style)


130-130: Code block style
Expected: fenced; Actual: indented

(MD046, code-block-style)


137-137: Code block style
Expected: fenced; Actual: indented

(MD046, code-block-style)


146-146: Code block style
Expected: fenced; Actual: indented

(MD046, code-block-style)


154-154: Code block style
Expected: fenced; Actual: indented

(MD046, code-block-style)


168-168: Code block style
Expected: fenced; Actual: indented

(MD046, code-block-style)


195-195: Code block style
Expected: fenced; Actual: indented

(MD046, code-block-style)


205-205: Code block style
Expected: fenced; Actual: indented

(MD046, code-block-style)


267-267: Code block style
Expected: fenced; Actual: indented

(MD046, code-block-style)


274-274: Code block style
Expected: fenced; Actual: indented

(MD046, code-block-style)


341-341: Unordered list indentation
Expected: 0; Actual: 2

(MD007, ul-indent)


342-342: Unordered list indentation
Expected: 0; Actual: 2

(MD007, ul-indent)


343-343: Unordered list indentation
Expected: 0; Actual: 2

(MD007, ul-indent)


344-344: Unordered list indentation
Expected: 0; Actual: 2

(MD007, ul-indent)


345-345: Unordered list indentation
Expected: 0; Actual: 2

(MD007, ul-indent)


346-346: Unordered list indentation
Expected: 0; Actual: 2

(MD007, ul-indent)


372-372: Heading style
Expected: atx; Actual: setext

(MD003, heading-style)


377-377: Bare URL used

(MD034, no-bare-urls)


378-378: Bare URL used

(MD034, no-bare-urls)


379-379: Bare URL used

(MD034, no-bare-urls)

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: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9f50ca2 and e61aa02.

📒 Files selected for processing (2)
  • docs/user-guide/migration-guide-v3.md (1 hunks)
  • mkdocs.yml (1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/user-guide/migration-guide-v3.md

[grammar] ~11-~11: There might be a mistake here.
Context: ...thoroughly. --- ## 💥 Breaking Changes ### Effect System Redesign Terminology chan...

(QB_NEW_EN)


[grammar] ~102-~102: There might be a mistake here.
Context: ...()` | --- ## 🗑️ Deprecated Parameters ??? abstract "InvestParameters" | O...

(QB_NEW_EN)


[grammar] ~206-~206: There might be a mistake here.
Context: ... 80% charged | --- ## 🔧 Common Issues | Issue | Solution | |-------|----------...

(QB_NEW_EN)


[style] ~233-~233: Using many exclamation marks might seem excessive (in this case: 10 exclamation marks for a text that’s 4509 characters long)
Context: ... !!! success "Welcome to flixopt v3.0.0! 🎉"

(EN_EXCESSIVE_EXCLAMATION)

🪛 markdownlint-cli2 (0.18.1)
docs/user-guide/migration-guide-v3.md

7-7: Link fragments should be valid

(MD051, link-fragments)


7-7: Link fragments should be valid

(MD051, link-fragments)


48-48: Table column count
Expected: 3; Actual: 4; Too many cells, extra data will be missing

(MD056, table-column-count)


49-49: Table column count
Expected: 3; Actual: 4; Too many cells, extra data will be missing

(MD056, table-column-count)


50-50: Table column count
Expected: 3; Actual: 4; Too many cells, extra data will be missing

(MD056, table-column-count)


51-51: Table column count
Expected: 3; Actual: 4; Too many cells, extra data will be missing

(MD056, table-column-count)


52-52: Table column count
Expected: 3; Actual: 4; Too many cells, extra data will be missing

(MD056, table-column-count)


53-53: Table column count
Expected: 3; Actual: 5; Too many cells, extra data will be missing

(MD056, table-column-count)


54-54: Table column count
Expected: 3; Actual: 4; Too many cells, extra data will be missing

(MD056, table-column-count)


211-211: Link fragments should be valid

(MD051, link-fragments)


224-224: Link fragments should be valid

(MD051, link-fragments)


224-224: Link fragments should be valid

(MD051, link-fragments)


226-226: Link fragments should be valid

(MD051, link-fragments)

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

Comment on lines +47 to 55
| Investment | `is_invested` | `invested` |
| Switching | `switch_on` | `switch|on` |
| Switching | `switch_off` | `switch|off` |
| Switching | `switch_on_nr` | `switch|count` |
| Effects | `Effect(invest)|total` | `Effect(periodic)` |
| Effects | `Effect(operation)|total` | `Effect(temporal)` |
| Effects | `Effect(operation)|total_per_timestep` | `Effect(temporal)|per_timestep` |
| Effects | `Effect|total` | `Effect` |

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

Escape | in table cells to keep column structure intact.

The “Variable Names” table still renders with misaligned columns because the literal | characters inside the new names aren’t escaped, so Markdown treats them as extra separators (see MD056 reports and broken layout in preview). Escape the pipes (or wrap with <code> tags) so the migration guide stays copy/paste-safe.

-| Switching                       | `switch_on` | `switch|on` |
-| Switching                       | `switch_off` | `switch|off` |
-| Switching                       | `switch_on_nr` | `switch|count` |
-| Effects                         | `Effect(invest)|total` | `Effect(periodic)` |
-| Effects        | `Effect(operation)|total` | `Effect(temporal)` |
-| Effects | `Effect(operation)|total_per_timestep` | `Effect(temporal)|per_timestep` |
-| Effects                    | `Effect|total` | `Effect` |
+| Switching                       | `switch_on` | `switch\|on` |
+| Switching                       | `switch_off` | `switch\|off` |
+| Switching                       | `switch_on_nr` | `switch\|count` |
+| Effects                         | `Effect(invest)\|total` | `Effect(periodic)` |
+| Effects        | `Effect(operation)\|total` | `Effect(temporal)` |
+| Effects | `Effect(operation)\|total_per_timestep` | `Effect(temporal)\|per_timestep` |
+| Effects                    | `Effect\|total` | `Effect` |
📝 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
| Investment | `is_invested` | `invested` |
| Switching | `switch_on` | `switch|on` |
| Switching | `switch_off` | `switch|off` |
| Switching | `switch_on_nr` | `switch|count` |
| Effects | `Effect(invest)|total` | `Effect(periodic)` |
| Effects | `Effect(operation)|total` | `Effect(temporal)` |
| Effects | `Effect(operation)|total_per_timestep` | `Effect(temporal)|per_timestep` |
| Effects | `Effect|total` | `Effect` |
| Investment | `is_invested` | `invested` |
| Switching | `switch_on` | `switch\|on` |
| Switching | `switch_off` | `switch\|off` |
| Switching | `switch_on_nr` | `switch\|count` |
| Effects | `Effect(invest)\|total` | `Effect(periodic)` |
| Effects | `Effect(operation)\|total` | `Effect(temporal)` |
| Effects | `Effect(operation)\|total_per_timestep` | `Effect(temporal)\|per_timestep` |
| Effects | `Effect\|total` | `Effect` |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)

48-48: Table column count
Expected: 3; Actual: 4; Too many cells, extra data will be missing

(MD056, table-column-count)


49-49: Table column count
Expected: 3; Actual: 4; Too many cells, extra data will be missing

(MD056, table-column-count)


50-50: Table column count
Expected: 3; Actual: 4; Too many cells, extra data will be missing

(MD056, table-column-count)


51-51: Table column count
Expected: 3; Actual: 4; Too many cells, extra data will be missing

(MD056, table-column-count)


52-52: Table column count
Expected: 3; Actual: 4; Too many cells, extra data will be missing

(MD056, table-column-count)


53-53: Table column count
Expected: 3; Actual: 5; Too many cells, extra data will be missing

(MD056, table-column-count)


54-54: Table column count
Expected: 3; Actual: 4; Too many cells, extra data will be missing

(MD056, table-column-count)

🤖 Prompt for AI Agents
In docs/user-guide/migration-guide-v3.md around lines 47 to 55, the table
columns are broken because literal pipe characters in the "new names" column are
unescaped; update each affected cell to either escape the pipe as \| or wrap the
value in inline code/HTML (e.g. <code>...</code>) so the pipes are treated as
literal characters (e.g. change switch|on → switch\|on or
`<code>switch|on</code>` for all occurrences: switch|on, switch|off,
switch|count, Effect(invest)|total, Effect(operation)|total,
Effect(operation)|total_per_timestep, Effect|total, etc.) and re-run the
Markdown linter to ensure MD056 no longer reports errors.

@FBumann FBumann merged commit 552569a into main Oct 16, 2025
7 of 8 checks passed
@FBumann FBumann deleted the fix/update-migration-guide-and-changelog branch November 19, 2025 13:42
@coderabbitai coderabbitai bot mentioned this pull request Nov 25, 2025
9 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