Skip to content

[Fix #99] Convert wrscatter directions from degrees to radians - #348

Open
dylanpulver wants to merge 1 commit into
python-windrose:mainfrom
dylanpulver:fix-wrscatter-degrees
Open

[Fix #99] Convert wrscatter directions from degrees to radians#348
dylanpulver wants to merge 1 commit into
python-windrose:mainfrom
dylanpulver:fix-wrscatter-degrees

Conversation

@dylanpulver

@dylanpulver dylanpulver commented Sep 2, 2026

Copy link
Copy Markdown

wrscatter negates the direction while it is still in degrees and then adds np.radians(90), so the value handed to the polar axes is not an angle in radians at all:

direction = -np.array(direction) + np.radians(90)

The transform the rest of the library uses is theta = radians(90 - direction) - that is what the angles array in WindroseAxes._init_plot computes, and it is what bar, box, contour and contourf all plot against. Measured against that convention, on main:

compass   expected theta   plotted theta   error
      0           1.5708          1.5708     0.0 deg
     45           0.7854          0.5531   -13.3 deg
     90           0.0000          5.8186   -26.6 deg
    180           4.7124          3.7832   -53.2 deg
    270           3.1416          1.7478   -79.9 deg

Only 0 degrees is right, by coincidence. This is the same symptom as #99 ("all the polar plots have 0 angle at North, but wrscatter is drawn like a usual polar plot"): the 2018 fix for that issue added -wd + pi/2, which is the correct expression for wd in radians, but wrscatter is fed degrees. So this finishes that fix rather than changing the intended convention.

Also affects plot_windrose(..., kind="scatter"), since D_KIND_PLOT["scatter"] is wrscatter.

Three image baselines encoded the old placement and are regenerated: output/func/test_wrscatter.png, output/df/test_scatter.png, output/df/test_windrose_np_plot_and_pd_plot.png. Before you trust them: regenerating test_wrbar.png from unmodified main here reproduces the committed file with RMS 0.0000, so this machine's rendering matches whatever produced the current baselines, while the committed test_wrscatter.png was already RMS 5.76 away from what unmodified main draws here, against a tolerance of 10. After this PR that figure regenerates exactly.

Added test_wrscatter_direction_convention, which asserts the plotted theta against radians(90 - direction) rather than against a picture. Ran pytest -s -rxs -Werror tests/ --mpl on Python 3.14 / matplotlib 3.11.1 / macOS: 34 passed before, 35 passed after. Reverting only windrose/windrose.py fails the new test; so does writing radians(direction) without the sign flip. Writing it as -radians(direction) + pi/2 is algebraically the same and also passes - I used the radians(90 - d) form because it reads as the library's own convention.

Not touched: WindroseAxes.scatter itself, which tests/test_windrose_np_mpl_oo.py calls with raw degrees and no transform — a separate question, and changing it would break anyone already passing radians.

AI disclosure: drafted with Claude Opus 5 (claude-opus-5); the numbers above are from runs on this machine.

… radians

wrscatter negated the direction in degrees and then added pi/2 radians,
so a compass bearing landed at an essentially arbitrary angle. The
transform the rest of the library uses is theta = radians(90 - direction)
(see the `angles` array in WindroseAxes._init_plot); only 0 degrees came
out right by coincidence.

Regenerates the three scatter image baselines, which encoded the old
placement, and adds a numeric test for the direction convention.
@ocefpaf

ocefpaf commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

I'm not super familiar with the code, but consistency with the other plotting methods is definitely a plus! We would need to mint a new major release though b/c, in a way, it is a breaking change.

I'll leave this here to see if we can get more comments from others. If not I plan to take a look using the best of my knowledge of the code and merge.

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