FIX: errors in get_position changes #12363
Conversation
|
Maybe this is a good place to add a test for the failing usecase? |
|
New test fails master, passes this PR and was the root of the problem.... |
59a994b to
a5e6667
Compare
|
Please rebase. #12366 is in and should fix the failing CI. |
a5e6667 to
0ce489c
Compare
|
...rebased |
|
flake8 says
|
0ce489c to
1da1c35
Compare
|
Fixed flake8. I'd actually be in favour of adding an Exception for the trailing whitespace error. |
lib/matplotlib/figure.py
Outdated
| @@ -2298,7 +2298,8 @@ def get_tightbbox(self, renderer, bbox_extra_artists=None): | |||
|
|
|||
| for ax in self.axes: | |||
| if ax.get_visible(): | |||
| bb.append(ax.get_tightbbox(renderer, bbox_extra_artists)) | |||
| bb.append(ax.get_tightbbox(renderer, | |||
| bbox_extra_artists=bbox_extra_artists)) | |||
There was a problem hiding this comment.
The alignment of this keyword argument makes me think it is for the append, not the get_tightbbox(), but even that looks wonky. Which PEP8 alignment rule is this?
There was a problem hiding this comment.
I just did two-tab (8-space) indentation - I don't think it'll fit on one line, though I can check
There was a problem hiding this comment.
It will not, nor will it work with aligning to the correct opening parenthesis, but you can do this:
bb.append(
ax.get_tightbbox(renderer,
bbox_extra_artists=bbox_extra_artists)) There was a problem hiding this comment.
Ummm, OK, but really the 8-space indent is allowed by PEP8, and in this case I think looks best:
There was a problem hiding this comment.
I'm meant "will not fit", not anything about whether it's in PEP8.
There was a problem hiding this comment.
That PEP8 link uses 4-space indent for continuation lines; it's only if there's ambiguity that double indent is used (but keep in mind there are no nested function calls in those examples.)
There was a problem hiding this comment.
I think a list comprehension would be more readable anyway.
There was a problem hiding this comment.
OK, list comprehension is bearable.
There was a problem hiding this comment.
Uh, I think (hope?) @timhoffm meant something like
bbox.extend(
ax.get_tightbbox(renderer, bbox_extra_artists=bbox_extra_artists)
for ax in self.axes if ax.get_visible())
| for ax in self.parasites] | ||
| bbs.append(super().get_tightbbox(renderer, call_axes_locator)) | ||
| bbs.append(super().get_tightbbox(renderer, | ||
| call_axes_locator=call_axes_locator)) |
There was a problem hiding this comment.
similar issue here, the alignment of the keyword argument is wonky.
a518ddf to
d4c7069
Compare
TST: add image grid test
d4c7069 to
78db75f
Compare
| @@ -327,9 +327,10 @@ def _remove_method(h): | |||
| return ax2 | |||
|
|
|||
| def get_tightbbox(self, renderer, call_axes_locator=True): | |||
| bbs = [ax.get_tightbbox(renderer, call_axes_locator) | |||
| bbs = [ax.get_tightbbox(renderer, call_axes_locator=call_axes_locator) | |||
There was a problem hiding this comment.
I don't see how this change is needed? (doesn't hurt, but still?)
|
Owee, I'm MrMeeseeks, Look at me. There seem to be a conflict, please backport manually. Here are approximate instructions:
And apply the correct labels and milestones. Congratulation you did some good work ! Hopefully your backport PR will be tested by the continuous integration and merged soon! If these instruction are inaccurate, feel free to suggest an improvement. |
…tion-errors FIX: errors in get_position changes
…3.0.x Backport PR #12363 on branch v3.0.x
PR Summary
Closes #12355
This corrects a few mistakes in positioning PRs that were preventing locatable axes from working correctly. It also makes a couple of kwargs explicit rather than positional when we call them.
PR Checklist