feat!: remove all overdue deprecated APIs (Optimization/Results, FlowSystem methods, kwarg shims)#741
feat!: remove all overdue deprecated APIs (Optimization/Results, FlowSystem methods, kwarg shims)#741FBumann wants to merge 7 commits into
Conversation
Removes the pre-accessor workflow API that has outlived two removal deadlines (conftest said v6.0.0, DEPRECATION_REMOVAL_VERSION said 7.0.0): - Optimization and SegmentedOptimization classes (flixopt/optimization.py) - Results and per-element Results classes (flixopt/results.py) - FlowSystem.from_old_results() migration helper (itself deprecated) - normalize_weights deprecated parameter on create_model/build_model/optimize - dead supporting code: ResultsContainer, used_in_calculation flag - tests/deprecated (32 files) and the deprecated_api pytest marker The file-migration path is intentionally kept: FlowSystem.from_old_dataset() and the io conversion helpers still load pre-v5 files. BREAKING CHANGE: fx.Optimization, fx.SegmentedOptimization, flixopt.results and FlowSystem.from_old_results() are removed. Use FlowSystem.optimize() / build_model() + solve() and access results via FlowSystem.solution. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 30 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis change removes deprecated optimization, results, transformation, visualization, and parameter APIs for v8, replaces Changesv8 API cleanup and migration
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@flixopt/flow_system.py`:
- Around line 827-828: The from_old_dataset() migration API must remain
supported without deprecation warnings. Remove its v6 removal warning/notice,
and update related migration documentation references from the removed
from_old_results() API to from_old_dataset().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9c85eeea-da0a-4f15-9cf6-3d828ad55b35
📒 Files selected for processing (44)
.github/workflows/tests.yaml.gitignoreflixopt/__init__.pyflixopt/flow_system.pyflixopt/optimization.pyflixopt/optimize_accessor.pyflixopt/results.pyflixopt/structure.pypyproject.tomltests/conftest.pytests/deprecated/__init__.pytests/deprecated/conftest.pytests/deprecated/examples/00_Minmal/minimal_example.pytests/deprecated/examples/01_Simple/simple_example.pytests/deprecated/examples/02_Complex/complex_example.pytests/deprecated/examples/02_Complex/complex_example_results.pytests/deprecated/examples/03_Optimization_modes/example_optimization_modes.pytests/deprecated/examples/04_Scenarios/scenario_example.pytests/deprecated/examples/05_Two-stage-optimization/two_stage_optimization.pytests/deprecated/test_bus.pytests/deprecated/test_component.pytests/deprecated/test_config.pytests/deprecated/test_cycle_detection.pytests/deprecated/test_effect.pytests/deprecated/test_effects_shares_summation.pytests/deprecated/test_examples.pytests/deprecated/test_flow.pytests/deprecated/test_flow_system_resample.pytests/deprecated/test_functional.pytests/deprecated/test_heatmap_reshape.pytests/deprecated/test_integration.pytests/deprecated/test_io.pytests/deprecated/test_linear_converter.pytests/deprecated/test_network_app.pytests/deprecated/test_on_hours_computation.pytests/deprecated/test_plotting_api.pytests/deprecated/test_resample_equivalence.pytests/deprecated/test_results_io.pytests/deprecated/test_results_overwrite.pytests/deprecated/test_results_plots.pytests/deprecated/test_scenarios.pytests/deprecated/test_storage.pytests/deprecated/test_timeseries.pytests/io/test_io_conversion.py
💤 Files with no reviewable changes (37)
- tests/deprecated/init.py
- tests/deprecated/examples/01_Simple/simple_example.py
- tests/deprecated/examples/00_Minmal/minimal_example.py
- tests/deprecated/test_plotting_api.py
- tests/deprecated/test_functional.py
- tests/deprecated/test_scenarios.py
- tests/deprecated/test_results_overwrite.py
- tests/deprecated/test_io.py
- tests/deprecated/examples/02_Complex/complex_example_results.py
- pyproject.toml
- tests/deprecated/test_network_app.py
- tests/deprecated/test_examples.py
- tests/deprecated/examples/05_Two-stage-optimization/two_stage_optimization.py
- tests/deprecated/test_config.py
- tests/deprecated/test_cycle_detection.py
- tests/deprecated/test_effect.py
- tests/deprecated/test_bus.py
- tests/deprecated/examples/04_Scenarios/scenario_example.py
- tests/deprecated/test_on_hours_computation.py
- tests/deprecated/test_results_plots.py
- tests/deprecated/test_heatmap_reshape.py
- tests/deprecated/test_results_io.py
- tests/deprecated/examples/03_Optimization_modes/example_optimization_modes.py
- tests/conftest.py
- tests/deprecated/examples/02_Complex/complex_example.py
- tests/deprecated/test_resample_equivalence.py
- flixopt/optimize_accessor.py
- tests/deprecated/test_effects_shares_summation.py
- tests/deprecated/test_storage.py
- tests/deprecated/test_linear_converter.py
- tests/deprecated/test_flow_system_resample.py
- tests/deprecated/test_component.py
- tests/deprecated/test_flow.py
- tests/deprecated/conftest.py
- tests/deprecated/test_integration.py
- flixopt/optimization.py
- tests/io/test_io_conversion.py
It is the only remaining loader for pre-v5 files, so drop its deprecation warning and docstring notice, and point the v5 migration guide at it now that from_old_results() is gone. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
All of these have warned 'will be removed in v7.0.0' since before 7.0.0 shipped; their replacements are long established: - plot_network(), start_network_app(), stop_network_app(), network_infos() -> topology accessor (topology.plot/start_app/stop_app/infos) - sel(), isel(), resample() and the _dataset_* static helpers -> transform accessor (transform.sel/isel/resample) - FlowSystem.coords property - topology.plot_legacy() and the pyvis-based plot_network helper, plus the pyvis dependency itself Tests exercising the old entry points were adapted to the accessor API (resample/sel equivalence suites) or removed where they only tested the deprecation shims. BREAKING CHANGE: FlowSystem.sel/isel/resample/coords/plot_network/ start_network_app/stop_network_app/network_infos and topology.plot_legacy() are removed. Use flow_system.transform.* and flow_system.topology.*. pyvis is no longer a dependency. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Removes Interface._handle_deprecated_kwarg and its sole remaining use, the Bus constructor's excess_penalty_per_flow_hour -> imbalance_penalty_per_flow_hour rename. Docs examples still teaching the old name were updated. File-level conversion is unaffected: flixopt.io.PARAMETER_RENAMES still maps old names when loading pre-v5 files. BREAKING CHANGE: passing excess_penalty_per_flow_hour to Bus(...) now raises a TypeError instead of warning. Use imbalance_penalty_per_flow_hour. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- docs/user-guide/migration-guide-v8.md in the style of the v6/v7 guides, with old/new examples for every removed entry point - linked from mkdocs nav, the v7 guide, and the CHANGELOG header tip - hand-written Unreleased changelog section summarizing the removals Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…oval The removed from_old_results tests were the only ones loading the real pre-v5 fixture files; from_old_dataset was left without real-file proof. Add a parametrized load test over all 10 fixtures plus one end-to-end solve (objective pinned), and document in the v8 migration guide that the pre-v5 file bridge is planned for removal in v9. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Keep plot_network somehow |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
flixopt/flow_system.py (1)
1670-1670: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReplace stale PyVis exports with the retained Plotly export API.
topology.plot()now returnsPlotResult;path=is passed as Plotly layout kwargs, soflow_system.topology.plot(path=...)will not export HTML. Remove the removedplot_legacy()reference and usePlotResult.to_html(...):
docs/home/quick-start.mddocs/user-guide/optimization/index.mdflixopt/topology_accessor.py: examples andSee Alsoflixopt/flow_system.py: examples andSee AlsoUse
flow_system.topology.plot(show=True).to_html('...').🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@flixopt/flow_system.py` at line 1670, Replace stale PyVis and plot_legacy references with the retained Plotly API: in flixopt/flow_system.py (1670-1670) and flixopt/topology_accessor.py (495-495), plus examples and See Also links in docs/home/quick-start.md and docs/user-guide/optimization/index.md (line ranges not provided), use flow_system.topology.plot(show=True).to_html('...') for HTML export and update references accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/user-guide/mathematical-notation/effects-and-dimensions.md`:
- Line 80: Remove the obsolete shortage_penalty_per_flow_hour keyword from the
Bus example, leaving imbalance_penalty_per_flow_hour as the single penalty
parameter for both imbalance directions.
In `@docs/user-guide/migration-guide-v8.md`:
- Line 30: Update the migration guide row for removed normalize_weights
arguments to document all affected APIs: optimize(...), create_model(), and
build_model().
In `@docs/user-guide/optimization/index.md`:
- Around line 366-371: Correct the documentation around
imbalance_penalty_per_flow_hour to state that its default is None, while
identifying 1e5 as the explicitly configured diagnostic penalty value in the
heat_bus example.
---
Outside diff comments:
In `@flixopt/flow_system.py`:
- Line 1670: Replace stale PyVis and plot_legacy references with the retained
Plotly API: in flixopt/flow_system.py (1670-1670) and
flixopt/topology_accessor.py (495-495), plus examples and See Also links in
docs/home/quick-start.md and docs/user-guide/optimization/index.md (line ranges
not provided), use flow_system.topology.plot(show=True).to_html('...') for HTML
export and update references accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5bb046c2-1e2b-45e2-a3f2-8e75652ee9fd
📒 Files selected for processing (19)
CHANGELOG.mddocs/user-guide/glossary.mddocs/user-guide/mathematical-notation/effects-and-dimensions.mddocs/user-guide/migration-guide-v7.mddocs/user-guide/migration-guide-v8.mddocs/user-guide/optimization/index.mdflixopt/elements.pyflixopt/flow_system.pyflixopt/plotting.pyflixopt/structure.pyflixopt/topology_accessor.pymkdocs.ymlpyproject.tomltests/flow_system/test_flow_system_resample.pytests/flow_system/test_resample_equivalence.pytests/io/test_io_conversion.pytests/plotting/test_network_app.pytests/plotting/test_topology_accessor.pytests/test_scenarios.py
💤 Files with no reviewable changes (5)
- flixopt/elements.py
- tests/plotting/test_network_app.py
- tests/plotting/test_topology_accessor.py
- pyproject.toml
- flixopt/structure.py
…penalty default - effects-and-dimensions: drop the nonexistent shortage_penalty_per_flow_hour kwarg; imbalance_penalty_per_flow_hour covers both directions - optimization guide: 1e5 is the example's diagnostic penalty, the default is None (strict balance) - migration guide: normalize_weights row lists all three affected entry points - topology docstring + results guide: replace plot_legacy/PyVis instructions with the Plotly Sankey API and write_html export - glossary: fold the shortage_penalty row into imbalance_penalty Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Collected into the |
Squash of the remove-deprecated-api branch. Removes everything that outlived its promised removal deadline (announced for v6/v7): - Optimization / SegmentedOptimization / Results classes - FlowSystem.sel/isel/resample/coords, plot_network and the network app wrappers, topology.plot_legacy() and the pyvis renderer (pyvis dependency dropped) - normalize_weights parameter, the deprecated-kwarg rename bridge, from_old_results(), tests/deprecated and the deprecated_api marker Kept: FlowSystem.from_old_dataset() + io conversion helpers as the supported pre-v5 file migration path, with real-file regression tests (sunset: v9). Adds docs/user-guide/migration-guide-v8.md and the CHANGELOG Unreleased section. BREAKING CHANGE: see the v8 migration guide for the complete removed-> replacement table. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Summary
Removes every API that has outlived its promised removal deadline (
tests/deprecated/conftest.pysaid v6.0.0,DEPRECATION_REMOVAL_VERSIONsays 7.0.0 — main is on 7.2.1).Net: −14,215 lines across 4 commits.
Commit 1 — Optimization/Results workflow API
Optimization/SegmentedOptimization,Results+ per-element results classesFlowSystem.from_old_results()(itself deprecation-warned)normalize_weightsdeprecated parameter oncreate_model/build_model/optimize()tests/deprecated/(32 files, ~9.2k lines), thedeprecated_apipytest marker, dead supporting codeCommit 2 — from_old_dataset() stays supported
The one remaining pre-v5 file loader keeps working without a deprecation warning; the v5 migration guide now points at it.
Commit 3 — deprecated FlowSystem methods + pyvis
plot_network(),start_network_app(),stop_network_app(),network_infos()→topology.*sel(),isel(),resample(),_dataset_*helpers,coordsproperty →transform.*topology.plot_legacy()and the pyvis renderer — pyvis is no longer a dependencyCommit 4 — kwarg rename bridge
Interface._handle_deprecated_kwargand its sole use:Bus(excess_penalty_per_flow_hour=...)now raisesTypeError— useimbalance_penalty_per_flow_hour. Docs examples still teaching the old name were fixed.Commit 5 — docs & release notes
docs/user-guide/migration-guide-v8.md(old/new examples for every removed entry point), linked from the mkdocs nav, the v7 guide, and the CHANGELOG header[Unreleased]CHANGELOG section — fold under the v8.0.0 heading when release-please cuts itDeliberately kept
FlowSystem.from_old_dataset()+flixopt.ioconversion helpers (PARAMETER_RENAMES, …): loading pre-v5 files remains supported, with tests.Migration
fx.Optimization(name, fs).solve(solver)fs.optimize(solver)Results/results.flow_rates()fs.solution,fs.statsfs.sel/isel/resample(...)fs.transform.sel/isel/resample(...)fs.plot_network()/ network appfs.topology.plot()/topology.start_app()Bus(excess_penalty_per_flow_hour=...)Bus(imbalance_penalty_per_flow_hour=...)Context
Peeled out of the element-data-classes stack (#602) so the overdue cleanup lands on its own timeline. This is the v8.0.0 trigger commit-wise; release-please will accumulate until the release PR is merged.
Full suite locally: 1183 passed, 0 failed.
🤖 Generated with Claude Code
Summary by CodeRabbit
Breaking Changes
Improvements
Documentation