Fix radial gridlines and outer spine collapsing on polar plots (#20388, #26972)#31701
Open
SharadhNaidu wants to merge 1 commit into
Open
Fix radial gridlines and outer spine collapsing on polar plots (#20388, #26972)#31701SharadhNaidu wants to merge 1 commit into
SharadhNaidu wants to merge 1 commit into
Conversation
9828bb6 to
6f8613d
Compare
Path.arc previously unwrapped the angular span to fit within 360 degrees, which silently collapsed near-but-not-quite-full-circle spans caused by floating-point noise (delta = 360 + epsilon) into near-empty arcs. This dropped the outer spine in several polar configurations and skipped radial gridlines drawn via the chunking loop in PolarTransform.transform_path_non_affine. Add an unwrap_angles keyword-only parameter to Path.arc (default True, preserving existing behaviour). PolarTransform's chunking loop is preserved unchanged; only the final partial-chunk Path.arc call uses unwrap_angles=False, which is exactly where the FP edge case triggers. Spine._adjust_location's polar branch also passes unwrap_angles=False so the outer spine is no longer collapsed when set_theta_offset is set. Closes matplotlib#20388 Closes matplotlib#26972
6f8613d to
3e7188b
Compare
Author
Visual before/afterHosted on a side branch of my fork so this comment doesn't blow up the PR diff. #20388 — #26972 — The bug only triggers on specific parameter values where the computed angular span lands on Note on
|
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.




This is a fix for two related polar plot bugs that have been hanging around for a while: #20388 (radial grid disappears with certain
set_theta_direction(-1)+set_theta_zero_locationcombinations) and #26972 (outer spine outline disappears whenset_theta_offsetis used). They look like separate problems but they're caused by the same thing.What's actually going on
Path.arc(theta1, theta2)unwraps the requested angular span into[0, 360]:That's the right thing for most callers. The problem is that the polar code paths (
PolarTransform.transform_path_non_affineandSpine._adjust_locationfor the polar spine) build uplow/highfromdirection * limit + offset. When the resulting span is supposed to be exactly 360°, floating-point arithmetic often makes it360 + tiny_epsiloninstead. Path.arc then unwraps that to a near-zero span and the full circle quietly collapses into a near-empty arc - so the grid or spine just vanishes.The chunking loop in
PolarTransform.transform_path_non_affine(while td - last_td > 360: ...) was there to work around this, but the chunking threshold and Path.arc's unwrap threshold use slightly different floating-point comparisons. When the difference falls in that narrow gap, you end up with onePath.arccall that draws basically nothing.anntzer did the bisect on #20388 and identified this exact mechanism. timhoffm replied on the same thread suggesting the fix:
That's what this PR does.
Changes
lib/matplotlib/path.py- add anunwrap_angles=Truekeyword-only parameter toPath.arc. Default is True so existing callers see no behaviour change at all. When set to False,theta1/theta2are used directly, which lets callers produce multi-turn arcs and avoid the floating-point edge case.lib/matplotlib/projections/polar.py- the constant-radius arc branch now callsPath.arc(..., unwrap_angles=False)directly instead of using the chunking loop. I verified the output is byte-identical to the old code for every common case (full circles, partial arcs, reversed arcs, near-360 gridlines).lib/matplotlib/spines.py- the polar outer spine now also passesunwrap_angles=False. This is the second place the bug manifests and is what makes [Bug]: set_theta_offset removes grid outline #26972 go away too.test_path.py(covers the new parameter and the floating-point edge case) andtest_polar.py(regression tests for both Radial grid missing in polar plots with ax.set_theta_direction(-1) and ax.set_theta_zero_location #20388 and [Bug]: set_theta_offset removes grid outline #26972, parametrised across multiple angles/offsets).next_whats_newentry for the new parameter.path.pyiupdated to match the new signature so stubtest stays happy.A few things worth flagging
I went through the polar baseline images and none of the scenarios they cover happen to hit a buggy parameter combo, so this PR doesn't need any baseline regenerations. I checked
polar_theta_positionin particular since('NW', 30, clockwise)looked suspicious - it's fine, spine spread is ~373 px (not collapsed).I'm aware of #26991 which targets #26972 with a different approach (tolerance-based comparison inside Path.arc itself). That approach would also work but it changes behaviour for every Path.arc caller; the explicit-flag approach here keeps the default exactly the same and only opts in the two polar code paths. Happy to discuss if reviewers prefer the other direction.
Tests
I'd recommend running
lib/matplotlib/tests/test_polar.py::test_polar_inverted_theta_outer_spineandtest_polar_theta_offset_outer_spineto see the regression tests for both issues. The newtest_arc_unwrap_anglesintest_path.pycovers the API addition directly.Closes #20388
Closes #26972
AI Disclosure
AI was used to help proofread the grammar of this PR description and to understand some parts of the existing codebase during development. The code changes and design decisions were developed independently.