BUG: Add wraparound logic for wind direction in environment plots#939
Open
khushalkottaru wants to merge 8 commits intoRocketPy-Team:developfrom
Open
BUG: Add wraparound logic for wind direction in environment plots#939khushalkottaru wants to merge 8 commits intoRocketPy-Team:developfrom
khushalkottaru wants to merge 8 commits intoRocketPy-Team:developfrom
Conversation
Gui-FernandesBR
force-pushed
the
daily-issue-scout-12924607668829467013
branch
from
57d5335 to
bb7b2e3
Compare
There was a problem hiding this comment.
Pull request overview
Updates RocketPy’s environment wind-direction plotting to avoid misleading lines when the direction wraps at the 360°→0° boundary, and adds an integration test intended to cover this scenario.
Changes:
- Insert NaNs into wind-direction plot data where direction jumps indicate 360°→0° wraparound, preventing matplotlib from drawing a connecting line.
- Apply the same wraparound handling in ensemble member wind-direction comparison plots.
- Add an integration test case that sets up a custom atmosphere with a direction wraparound and exercises plotting.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
rocketpy/plots/environment_plots.py |
Breaks wind-direction lines at wraparound points by inserting NaNs (single and ensemble plots). |
tests/integration/environment/test_environment.py |
Adds a regression-style integration test that sets up a wraparound wind direction and calls plotting APIs. |
You can also share your feedback on Copilot code review. Take the survey.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Member
|
@khushalkottaru great fix, thanks. Can you show us some plots of before and after implementation please? Just to make sure your PR descriptions matches the expected behavior. Btw I believe this is a related issue: #253 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull request type
Checklist
black rocketpy/ tests/) has passed locallypytest tests -m slow --runslow) have passed locallyCurrent behavior
When wind direction crosses the 0/360 boundary, matplotlib connects those two points with a straight line. This leads to cluttered and visually misleading graphs where the wind wrapped around.
New behavior
Now, before passing data to the plotting function, the code detects any consecutive pair of direction values that differ by more than 180 deg. If detected, a NaN is inserted at that position. This can be thought of as "lifting the pen" when drawing the graph. The actual data points are still plotted correctly, but the lines from before do not make an appearance.
Breaking change