Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Mar 29, 2025
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
88a4424
Fix #29528 set_rticks makes autoscale move the origin away from zero
bruswaine Mar 20, 2025
5964c42
Merge branch 'matplotlib:main' into bugfix-for-issue-29528
bruswaine Mar 24, 2025
273dff3
Fix #29528 set_rticks makes autoscale move the origin away from 0.
bruswaine Mar 25, 2025
c142cfa
Fix #29528 set_rticks makes autoscale move the origin away from 0.
bruswaine Mar 25, 2025
e9817d2
Update lib/matplotlib/tests/test_polar.py
bruswaine Mar 25, 2025
faa0515
Preserve set_yticks return value
bruswaine Mar 25, 2025
42f1d0b
Update lib/matplotlib/tests/test_polar.py
bruswaine Mar 29, 2025
2d7541c
fixed failing checks
bruswaine Mar 29, 2025
52107b8
Account for 0 margins in tests
bruswaine Mar 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion lib/matplotlib/projections/polar.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1292,7 +1292,10 @@ def set_rscale(self, *args, **kwargs):
return Axes.set_yscale(self, *args, **kwargs)

def set_rticks(self, *args, **kwargs):
return Axes.set_yticks(self, *args, **kwargs)
result = Axes.set_yticks(self, *args, **kwargs)
self.yaxis.set_major_locator(
self.RadialLocator(self.yaxis.get_major_locator(), self))
Comment on lines +1296 to +1297
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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()

image
image
image

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')

image

return result

def set_thetagrids(self, angles, labels=None, fmt=None, **kwargs):
"""
Expand Down
20 changes: 20 additions & 0 deletions lib/matplotlib/tests/test_polar.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -506,3 +506,23 @@ def test_polar_errorbar(order):
ax.errorbar(theta, r, xerr=0.1, yerr=0.1, capsize=7, fmt="o", c="seagreen")
ax.set_theta_zero_location("N")
ax.set_theta_direction(-1)


def test_radial_limits_behavior():
# r=0 is kept as limit if positive data and ticks are used
# negative ticks or data result in negative limits
fig = plt.figure()
ax = fig.add_subplot(projection='polar')
assert ax.get_ylim() == (0, 1)
# upper limit is expanded to include the ticks, but lower limit stays at 0
ax.set_rticks([1, 2, 3, 4])
assert ax.get_ylim() == (0, 4)
# upper limit is autoscaled to data, but lower limit limit stays 0
ax.plot([1, 2], [1, 2])
assert ax.get_ylim() == (0, 2)
# negative ticks also expand the negative limit
ax.set_rticks([-1, 0, 1, 2])
assert ax.get_ylim() == (-1, 2)
# negative data also autoscales to negative limits
ax.plot([1, 2], [-1, -2])
assert ax.get_ylim() == (-2, 2)
Loading