FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Ensure polar plot radial lower limit remains at 0 after set_rticks + plot by bruswaine · Pull Request #29798 · matplotlib/matplotlib · GitHub

  1. matplotlib
  2. matplotlib

Repository navigation

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
rcomer merged 9 commits into from
Mar 29, 2025
Conversation 17 Commits 9 Checks 0 Files changed
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 .py  (2) All 1 file type selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
5 changes: 4 additions & 1 deletion lib/matplotlib/projections/polar.py
Show comments
View file
Open in desktop
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
Show hidden characters
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 thread
rcomer marked this conversation as resolved.
Comment on lines +1296 to +1297

timhoffm Mar 28, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

rcomer Mar 29, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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')
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
Show comments
View file
Open in desktop
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
Show hidden characters
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)
Back | FazBrowse Home | New Git URL