-
-
Notifications
You must be signed in to change notification settings - Fork 235
ENH: replace if elif else chains with match statement #921
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ENH: replace if elif else chains with match statement #921
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors if/elif/else chains to use Python 3.10's match-case pattern matching syntax across multiple files and updates the ruff target version to Python 3.10. While most conversions are correct, there are critical logic errors in the refactoring that change the behavior of the code.
- Converted if/elif/else chains to match-case statements in 7 files
- Updated ruff target version from py39 to py310
- Introduced critical bug in environment.py from_dict method that changes execution logic
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pyproject.toml | Updated ruff target version from py39 to py310 |
| rocketpy/units.py | Converted axis label matching (0/1) to match-case |
| rocketpy/rocket/rocket.py | Converted coordinate system orientation validation to match-case |
| rocketpy/rocket/aero_surface/nose_cone.py | Converted nose cone kind validation to match-case with OR patterns |
| rocketpy/plots/rocket_plots.py | Converted plane coordinate mapping to match-case (2 functions) |
| rocketpy/motors/motor.py | Converted coordinate system orientation validation to match-case |
| rocketpy/mathutils/function.py | Converted interpolation/extrapolation type selection to match-case; incomplete refactoring in plot_2d method; missing default case in differentiate |
| rocketpy/environment/environment.py | Converted atmospheric model type handling to match-case; critical bug in from_dict where code moved outside match statement |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Gui-FernandesBR
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tibisabau can you update CHANGELOG.md file please?
|
@tibisabau we need all the tests to be green on CI |
Co-authored-by: Gui-FernandesBR <[email protected]>
Gui-FernandesBR
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM,
I will merge it if all tests pass.
@tibisabau could you run slow tests and share the evidence here?
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #921 +/- ##
===========================================
+ Coverage 80.27% 81.09% +0.82%
===========================================
Files 104 107 +3
Lines 12769 13833 +1064
===========================================
+ Hits 10250 11218 +968
- Misses 2519 2615 +96 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@tibisabau please fix linters so we can merge this PR |
|
amazing implementation! |
Summary
This PR refactors if/elif/else chains to use Python 3.10's
match-casepattern matching syntax, improving code readability and maintainability. Closes #725.This code was generated using Claude Sonnet 4.5.
Changes
if/elif/elsechains withmatch-casestatements.Pull request type