FIX: snap near-integer arc windings to a full circle on polar plots (#20388, #26972)#31844
Open
SharadhNaidu wants to merge 2 commits into
Open
FIX: snap near-integer arc windings to a full circle on polar plots (#20388, #26972)#31844SharadhNaidu wants to merge 2 commits into
SharadhNaidu wants to merge 2 commits into
Conversation
matplotlib#26972) On polar plots, radial gridlines and the outer spine could collapse to a near-empty arc. The root cause is Path.arc's angle-unwrap step: a span of 360*n plus a tiny floating-point rounding error was reduced modulo 360 to a near-zero arc instead of a full circle. Detect spans that are within floating-point tolerance of a whole number of turns and draw a complete circle; otherwise unwrap theta2 to the shortest arc within 360 degrees as before. The change is byte-identical for all non-degenerate inputs, so no call sites need to change. Adds unit coverage for the snap/unwrap behaviour and end-to-end polar gridline and spine regression tests.
- Replace the 1e-9 magic tolerance with 1e-12 turns, justified by the observed error from the polar transform pipeline (machine-epsilon level, under 1e-15 turns), via np.rint(n_turns). - Snap on any non-zero whole number of turns (abs winding), restoring the legacy full-circle behaviour for exact negative multiples such as Path.arc(0, -360); the previous rework collapsed those to an empty arc. - Add regression tests for negative full circles and a tolerance-tightness guard (a span just past a full turn must not snap).
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.
PR summary
On polar plots, radial gridlines (#20388) and the outer spine (#26972) could collapse to a near-empty arc for certain
set_theta_zero_location/set_theta_offset/set_theta_directioncombinations.The root cause is in
Path.arc's angle-unwrap step. A span of360*nplus a tiny floating-point rounding error was reduced modulo 360 to a near-zero arc instead of a full circle:Fix
Detect spans that are within floating-point tolerance of a whole number of turns and draw a complete circle; otherwise unwrap
theta2to the shortest arc within 360 degrees exactly as before:The change is byte-identical for all non-degenerate inputs (e.g. 90°, 359.99°, 360°, 410°→50°, 540°→180°, 720°→full circle) and only affects the
360*n + εcollapse cases, so no call sites need to change.Closes #20388
Closes #26972
Tests
test_arc_full_circle_snap/test_arc_unwrap_partial_turnintest_path.pycover the snap-vs-unwrap behaviour at the unit level.test_path.pyand end-to-end polar gridline / outer-spine regression tests intest_polar.py.PR checklist
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.