-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Fix polar get window extent #13614
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.
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
Fix polar get window extent #13614
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Repeat my comments for the remaining figures.
@@ -20,6 +20,7 @@ | ||
|
||
""" | ||
|
||
from __future__ import division |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need this unless you're backported to 2.2.x?
ax = [] | ||
fig1, axs = plt.subplots(rows, cols, figsize=figsize, constrained_layout=True) | ||
axs = axs.flat | ||
axs[-1].remove() # not used... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That depends on the length of cases, doesn't it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, but...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
something like for ax in axs[:len(cases)]: ax.remove()
after converting to axs.flat looks better to me, perhaps have a helper function.
ax.append(fig1.add_subplot(gs[row, col])) | ||
ax[-1].set_title('markevery=%s' % str(case)) | ||
ax[-1].plot(x, y, 'o', ls='-', ms=4, markevery=case) | ||
ax = axs[i] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for ax, case in zip(axs, cases):
(don't think i
is used for anything here now.)
fig4 = plt.figure(num=4, figsize=figsize) | ||
axpolar = [] | ||
fig4, axs = plt.subplots(rows, cols, figsize=figsize, | ||
subplot_kw = {'projection':'polar'}, constrained_layout=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove space around =
.
Added a test that fails on master:
and passes w/ this PR.... |
75cd5fd
to
bda7c19
Compare
@tacaswell re-milestoned, but feel free to re-milestone back. Maybe my mistake for mixing two things in the PR, but this fixes a real bug in getting the tightbbox from polar axes... |
bda7c19
to
e297810
Compare
…614-on-v3.1.x Backport PR #13614 on branch v3.1.x (Fix polar get window extent)
PR Summary
This PR does 2 things. First, cleans up
markerevery_demo
to use modern semantics; Second, catches an error inspines.get_window_extent
that can happen if the spine has no axis.PR Checklist