Ensure polar plot radial lower limit remains at 0 after set_rticks + plot #29798

bruswaine · 2025-03-24T16:11:16Z

Update: The final implementation differs from the original proposal. The solution now preserves the RadialLocator behavior that was being overwritten on set_rticks instead of modifying handle_single_axis. The test was renamed to test_radial_limits_behavior.

PR summary

Why is this change necessary?
When using set_rticks followed by plot in polar plots, the lower radial limit (which should
always be 0) was being overwritten by autoscaling logic. This caused visual artifacts and
incorrect axis behavior in polar plots.
What problem does it solve?
This fix prevents the radial origin from moving away from 0 when setting radial ticks with
set_rticks and adding data with plot.
What is the reasoning for this implementation?
- Modified handle_single_axis in autoscale_view to explicitly enforce lower limit of 0 for polar plots
- Added test test_radial_limit_after_plot to verify limit stays at 0 after set_rticks + plot

Example:

fig = plt.figure()
ax = fig.add_subplot(projection='polar')
ax.set_rticks([1, 2, 3, 4])  # Would previously allow lower limit > 0
ax.plot([0, 1], [2, 3])      # Now maintains lower limit at 0

Closes #29528

PR checklist

"closes [Bug]: set_rticks makes polar autoscale move the origin away from zero #29528" is in the body of the PR description to link the related issue
new and changed code is tested
[N/A] Plotting related features are demonstrated in an example
[N/A] New Features and API Changes are noted with a directive and release note
[N/A] Documentation complies with general and docstring guidelines

…from zero After calling `set_rticks`, the lower radial limit was being overwritten when `plot` was called. This was due to `autoscale_view` logic not enforcing the constraint that the lower radial limit must be 0 in polar plots. This fix ensures that the lower radial limit is always enforced by explicitly setting it to 0 inside `handle_single_axis` (inside `autoscale_view`). Added `test_radial_limit_after_plot` to `test_polar.py` to assert that the lower limit of the radial axis is 0.

github-actions

Thank you for opening your first PR into Matplotlib!

If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks.

You can also join us on gitter for real-time discussion.

For details on testing, writing docs, and our review process, please see the developer guide

We strive to be a welcoming and open project. Please follow our Code of Conduct.

rcomer · 2025-03-24T20:21:31Z

Thank you @bruswaine for your work on this. However, this is not quite the right fix. As shown by the failing tests, we do expect the radial lower limit to sometimes be negative, and sometimes r is on a log-scale.

bruswaine · 2025-03-24T22:18:23Z

Thanks @rcomer for the quick reply.

I've seen your comment on the Issue where you show the view_limits function.
Would adding more checks for those cases suffice for the fix? For instancex0 = min(0, x0) instead of just x0 = 0.
I've also noticed that some functions call the super and then set a RadialLocator.That would that be a cleaner solution, as it would keep all the logic already set for polar plots. I will investigate this further and test it out.

…from 0. After calling `set_rticks`, the lower radial limit was being overwritten when `plot` was called. This was due to `set_rticks` overwriting the `RadialLocator` wrapper for a `FixedLocator` losing all Radial logic. This fix sets the RadialLocator inside `set_rticks` after calling `Axes.set_yticks` ensuring all radial logic is applied. Added `test_radial_limits_behavior` to `test_polar.py` to assert that the lower limit of the radial axis is maintained after `set_rticks`.

lib/matplotlib/tests/test_polar.py

Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>

bruswaine · 2025-03-25T17:24:08Z

@timhoffm Note this PR also incidentally fixes #29516 (Cannot draw arrow from polar plot origin after setting rticks
As it's related to the origin being away from zero after set_rticks i didn't include any test for this specific issue but let me know if it's needed, i'd be happy to.

rcomer · 2025-03-25T18:21:47Z

@timhoffm Note this PR also incidentally fixes #29516 (Cannot draw arrow from polar plot origin after setting rticks As it's related to the origin being away from zero after set_rticks i didn't include any test for this specific issue but let me know if it's needed, i'd be happy to.

That issue was caused by a combination of two problems, and you fixed one of them. To close that issue, we should also fix the examples given at #29516 (comment).

bruswaine · 2025-03-25T18:29:16Z

@timhoffm Thanks for the clarification.
Sorry for my lack of knowledge as this is my first contribution to an open source project, is there anything else i should do after i accepted your suggested changes?

lib/matplotlib/projections/polar.py

rcomer

I’m pretty sure the Appveyor failure is unrelated.

timhoffm · 2025-03-28T23:51:48Z

lib/matplotlib/projections/polar.py

+        self.yaxis.set_major_locator(
+            self.RadialLocator(self.yaxis.get_major_locator(), self))


While this solves the particuar issue, I'm unclear whether this is the right fix:

Is this the right place to fix this? Or would it be better to override RadialAxis.set_major_locator to always wrap in a RadialLocator?

Not an expert here, but why do we manually warp in a RadialLocator and not use _warp_locator_formatter()? The difference is that the latter sets self.isDefault_majloc = True, which apparently has to do with unit converters, but the purpose and correct handling is quite unclear to me.

I think that if someone calls set_major_locator directly then we should respect it and used what they passed. The RadialLocator is public so, if that is what they really need, they can wrap themselves before passing it. If we always wrap and for some reason they didn't want RadialLocator then I don't think they have a way around the auto-wrapping? Here's a slightly esoteric example:

import matplotlib.pyplot as plt import matplotlib.dates as mdates from matplotlib.projections.polar import RadialLocator import numpy as np dates = np.arange('2025-01-01', '2027-01-01', dtype='datetime64[D]') theta = np.linspace(0, np.pi * 4, 365 * 2) fig, ax = plt.subplots(subplot_kw=dict(projection='polar')) ax.plot(theta, dates) ax.set_title('Default Ticks') fig, ax = plt.subplots(subplot_kw=dict(projection='polar')) ax.yaxis.set_major_locator(mdates.YearLocator()) ax.plot(theta, dates) ax.set_title('Year Locator') fig, ax = plt.subplots(subplot_kw=dict(projection='polar')) ax.yaxis.set_major_locator(RadialLocator(mdates.YearLocator(), ax)) ax.plot(theta, dates) ax.set_title('Year Locator wrapped with RadialLocator') plt.show()

I think setting self.isDefault_majloc = True allows the locator to be replaced with one defined by the converter. If we have explicitly set the ticks, then I think we don't want that. I tried to demonstrate something about this with my date example, but if I set the ticks before calling plot and/or yaxis_date, then the logic that uses isDefault_majloc never got called and I don't have date formatting 😕

fig, ax = plt.subplots(subplot_kw=dict(projection='polar')) ax.set_rticks(dates[::200]) ax.yaxis_date() ax.plot(theta, dates) ax.set_title('Set ticks')

lib/matplotlib/tests/test_polar.py

Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>

lib/matplotlib/tests/test_polar.py

Co-authored-by: Ruth Comer <10599679+rcomer@users.noreply.github.com>

rcomer · 2025-03-29T20:26:48Z

Thanks @bruswaine and congratulations on your first open source PR 🎉 Hopefully we will hear from you again.

bruswaine · 2025-03-29T20:31:10Z

Thank you @rcomer and @timhoffm for your guidance throughout this PR! I really appreciated your clear feedback and patience. This was a great learning experience, and I'm looking forward to contributing more to Matplotlib in the future.

All the wrapping logic is now contained in RadialAxis Closes matplotlib#30164 and rearchitects matplotlib#29798.

bruswaine added 2 commits March 20, 2025 22:28

Merge branch 'matplotlib:main' into bugfix-for-issue-29528

5964c42

github-actions bot added the topic: axes label Mar 24, 2025

github-actions bot reviewed Mar 24, 2025

View reviewed changes

melissawm added this to First Time Contributors Mar 24, 2025

melissawm moved this to Needs review in First Time Contributors Mar 24, 2025

github-actions bot added the topic: polar label Mar 25, 2025

github-actions bot removed the topic: axes label Mar 25, 2025

timhoffm reviewed Mar 25, 2025

View reviewed changes

lib/matplotlib/tests/test_polar.py Outdated Show resolved Hide resolved

Update lib/matplotlib/tests/test_polar.py

e9817d2

Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>

rcomer reviewed Mar 25, 2025

View reviewed changes

lib/matplotlib/projections/polar.py Show resolved Hide resolved

Preserve set_yticks return value

faa0515

rcomer approved these changes Mar 25, 2025

View reviewed changes

rcomer added topic: ticks axis labels PR: bugfix Pull requests that fix identified bugs labels Mar 26, 2025

timhoffm reviewed Mar 28, 2025

View reviewed changes

timhoffm reviewed Mar 29, 2025

View reviewed changes

lib/matplotlib/tests/test_polar.py Outdated Show resolved Hide resolved

Update lib/matplotlib/tests/test_polar.py

42f1d0b

Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>

github-actions bot removed the topic: ticks axis labels label Mar 29, 2025

fixed failing checks

2d7541c

timhoffm approved these changes Mar 29, 2025

View reviewed changes

rcomer reviewed Mar 29, 2025

View reviewed changes

lib/matplotlib/tests/test_polar.py Outdated Show resolved Hide resolved

rcomer added this to the v3.11.0 milestone Mar 29, 2025

Account for 0 margins in tests

52107b8

Co-authored-by: Ruth Comer <10599679+rcomer@users.noreply.github.com>

rcomer merged commit 2933275 into matplotlib:main Mar 29, 2025
39 of 41 checks passed

github-project-automation bot moved this from Needs review to Waiting for author in First Time Contributors Mar 29, 2025

QuLogic moved this from Waiting for author to Merged in First Time Contributors Mar 30, 2025

anntzer mentioned this pull request Jun 12, 2025

[Bug]: Removing spines in polar plot causes distortion of the plot #30164

Open

timhoffm added a commit to timhoffm/matplotlib that referenced this pull request Jun 13, 2025

FIX: Ensure Locators on RadialAxis are always correctly wrapped

5c6d199

All the wrapping logic is now contained in RadialAxis Closes matplotlib#30164 and rearchitects matplotlib#29798.

timhoffm mentioned this pull request Jun 13, 2025

FIX: Ensure Locators on RadialAxis are always correctly wrapped #30174

Open

timhoffm added a commit to timhoffm/matplotlib that referenced this pull request Jun 16, 2025

FIX: Ensure Locators on RadialAxis are always correctly wrapped

a4806aa

All the wrapping logic is now contained in RadialAxis Closes matplotlib#30164 and rearchitects matplotlib#29798.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Ensure polar plot radial lower limit remains at 0 after set_rticks + plot #29798

Ensure polar plot radial lower limit remains at 0 after set_rticks + plot #29798

Uh oh!

bruswaine commented Mar 24, 2025 •

edited

Loading

Uh oh!

github-actions bot left a comment

Uh oh!

rcomer commented Mar 24, 2025 •

edited

Loading

Uh oh!

bruswaine commented Mar 24, 2025

Uh oh!

Uh oh!

bruswaine commented Mar 25, 2025 •

edited

Loading

Uh oh!

rcomer commented Mar 25, 2025 •

edited

Loading

Uh oh!

bruswaine commented Mar 25, 2025

Uh oh!

Uh oh!

rcomer left a comment

Uh oh!

timhoffm Mar 28, 2025

Uh oh!

rcomer Mar 29, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rcomer commented Mar 29, 2025

Uh oh!

bruswaine commented Mar 29, 2025

Uh oh!

Uh oh!

		self.yaxis.set_major_locator(
		self.RadialLocator(self.yaxis.get_major_locator(), self))

Uh oh!

Ensure polar plot radial lower limit remains at 0 after set_rticks + plot #29798

Ensure polar plot radial lower limit remains at 0 after set_rticks + plot #29798

Uh oh!

Conversation

bruswaine commented Mar 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR summary

PR checklist

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

rcomer commented Mar 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bruswaine commented Mar 24, 2025

Uh oh!

Uh oh!

bruswaine commented Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rcomer commented Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bruswaine commented Mar 25, 2025

Uh oh!

Uh oh!

rcomer left a comment

Choose a reason for hiding this comment

Uh oh!

timhoffm Mar 28, 2025

Choose a reason for hiding this comment

Uh oh!

rcomer Mar 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rcomer commented Mar 29, 2025

Uh oh!

bruswaine commented Mar 29, 2025

Uh oh!

Uh oh!

bruswaine commented Mar 24, 2025 •

edited

Loading

rcomer commented Mar 24, 2025 •

edited

Loading

bruswaine commented Mar 25, 2025 •

edited

Loading

rcomer commented Mar 25, 2025 •

edited

Loading