Improve filterwarnings #485
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds a centralized Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
flixopt/results.py (2)
2417-2422: Add version removal information to this deprecation warning.This deprecation warning is missing the version removal information that was added to other warnings in this PR. For consistency with lines 2268-2274 (which deprecate the same parameters) and the PR objective to add version info to all deprecations, this warning should also include the removal version.
Apply this diff:
warnings.warn( "The 'heatmap_timeframes' and 'heatmap_timesteps_per_frame' parameters are deprecated. " - "Use 'reshape_time=(timeframes, timesteps_per_frame)' instead.", + "Use 'reshape_time=(timeframes, timesteps_per_frame)' instead. " + f'Will be removed in v{DEPRECATION_REMOVAL_VERSION}.', DeprecationWarning, stacklevel=2, )
2436-2440: Add version removal information to this deprecation warning.This deprecation warning is missing the version removal information that was added to other warnings in this PR. For consistency with lines 2288-2293 (which deprecate the same parameter) and the PR objective to add version info to all deprecations, this warning should also include the removal version.
Apply this diff:
warnings.warn( - "The 'color_map' parameter is deprecated. Use 'colors' instead.", + f"The 'color_map' parameter is deprecated. Use 'colors' instead. " + f'Will be removed in v{DEPRECATION_REMOVAL_VERSION}.', DeprecationWarning, stacklevel=2, )
🧹 Nitpick comments (1)
flixopt/flow_system.py (1)
1109-1117: Consider updating the hardcoded version for consistency.The
all_elementsproperty deprecation warning uses a hardcoded version"v4.0.0"(line 1113), while this PR establishes a pattern of usingDEPRECATION_REMOVAL_VERSIONfor all other deprecation warnings.For consistency and maintainability, consider updating this to:
f'This property will be removed in v{DEPRECATION_REMOVAL_VERSION}.'Unless v4.0.0 is intentionally different from the value in
DEPRECATION_REMOVAL_VERSION, in which case this is fine as-is.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
CHANGELOG.md(1 hunks)flixopt/calculation.py(3 hunks)flixopt/effects.py(18 hunks)flixopt/elements.py(1 hunks)flixopt/flow_system.py(4 hunks)flixopt/results.py(9 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
flixopt/results.py (1)
flixopt/config.py (1)
CONFIG(169-700)
flixopt/effects.py (2)
flixopt/elements.py (1)
label_full(614-615)flixopt/structure.py (2)
label_full(984-985)label_full(1476-1477)
🔇 Additional comments (14)
CHANGELOG.md (1)
70-71: Clear documentation of the deprecation warning improvements.The changelog entry accurately describes the changes made to centralize deprecation version management. Good practice to document this refactoring.
flixopt/elements.py (1)
519-520: LGTM! Consistent deprecation message format.The addition of the removal version notice improves user communication by clearly indicating when this deprecated feature will be removed.
flixopt/flow_system.py (3)
18-18: LGTM! Import added to support centralized deprecation version management.This import enables the consistent use of
DEPRECATION_REMOVAL_VERSIONthroughout the module, improving maintainability.
998-999: Consistent deprecation message update.The warning message now clearly communicates when this deprecated behavior will be removed, improving user experience.
1169-1170: Well-coordinated deprecation warnings for the weights property.Both the getter and setter now include the removal version notice, ensuring consistent messaging for this deprecated property.
Also applies to: 1185-1186
flixopt/effects.py (3)
19-19: LGTM! Essential import for centralized deprecation management.This import enables consistent deprecation version messaging across all the Effect class properties.
256-257: Excellent systematic update of all deprecated Effect properties.All deprecated property getters and setters now consistently include the removal version notice. This comprehensive update ensures users have clear guidance across the entire Effect API surface.
Also applies to: 267-268, 278-279, 289-290, 300-301, 311-312, 322-323, 333-334, 344-345, 355-356, 366-367, 377-378, 388-389, 399-400, 410-411, 421-422
637-638: Consistent deprecation messaging for effect object usage.The warning for using Effect objects instead of labels now includes the removal version, maintaining consistency with the rest of the codebase.
flixopt/calculation.py (3)
28-28: LGTM! Import added for centralized deprecation version management.This import enables consistent deprecation messaging for the
active_timestepsparameter and property.
82-82: Clear deprecation messaging for the active_timesteps parameter.The warning now includes the removal version, helping users plan their migration to the recommended
flow_system.sel(time=...)orflow_system.isel(time=...)approach.
165-166: Well-coordinated deprecation warnings.Both the
active_timestepsparameter and property now include consistent removal version information, ensuring clear messaging regardless of how users access this deprecated feature.flixopt/results.py (3)
18-18: LGTM: Import statement is correct.The import of
DEPRECATION_REMOVAL_VERSIONfrom config is properly added.
230-236: LGTM: Deprecation warning properly updated.The deprecation message now includes the removal version, which helps users plan their migration.
1325-1330: LGTM: Consistent deprecation warning format.The
indexerparameter deprecation properly includes version removal information.
… heatmap_timesteps_per_frame deprecation warning to include version removal information using DEPRECATION_REMOVAL_VERSION 2. ✅ flixopt/results.py:2421-2426 - The color_map deprecation warning already had the version info (it was updated before I started) 3. ✅ flixopt/flow_system.py:1109-1116 - Updated the hardcoded "v4.0.0" to use DEPRECATION_REMOVAL_VERSION for consistency with all other deprecation warnings
Description
Add proper version to all deprecations
Type of Change
Related Issues
Closes #(issue number)
Testing
Checklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.