Skip to content

FIX: Wind rose bins were not properly covering the entire compass.#1001

Open
rcjackson wants to merge 3 commits intoARM-DOE:mainfrom
rcjackson:windrose_fix
Open

FIX: Wind rose bins were not properly covering the entire compass.#1001
rcjackson wants to merge 3 commits intoARM-DOE:mainfrom
rcjackson:windrose_fix

Conversation

@rcjackson
Copy link
Copy Markdown
Collaborator

  • PEP8 Standards or use of linter
  • Xarray Dataset or DataArray variable naming follows 'ds' or 'da' naming

A co-author on my WaggleDop paper noticed an issue with a missing bin in the WindRoseDisplay plots. It looks like the bins are not being set up correctly in the WindRoseDisplay object, as they are laid out in a way that would exclude the last division inside the wind rose. See here as an example of the current implementation:
wind_rose_plots_wref_missing_bin

When I put in these new bin centers, there is now a complete histogram throughout the entire compass:
wind_rose_plots_wref

Copy link
Copy Markdown
Collaborator

@zssherman zssherman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Thanks for the changes! The tests are failing due to noaapsl being down so ill rerun the tests soon.

@zssherman zssherman closed this Apr 20, 2026
@zssherman zssherman reopened this Apr 20, 2026
@zssherman
Copy link
Copy Markdown
Collaborator

@rcjackson Can you update the test file when you get a chance?

@zssherman
Copy link
Copy Markdown
Collaborator

@rcjackson test_plot.png has a windrose in it as well

@rcjackson
Copy link
Copy Markdown
Collaborator Author

rcjackson commented Apr 20, 2026 via email

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