-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
FIX: turn off title autopos if pad is set #16862
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
Conversation
What should happen if the user specifies the padding in the rc paramters, i.e. |
It would still auto position. I guess I think the pad of 6 is a mistake, and it should be a pad of None, and resolved to 6 when needed. Thats a bit more of a change, but I don't think its too much work. |
Looked into this. Its pretty easy to just change the title's y to be above the top decorations and not have any logic attached to However, that gives perhaps unexpected results if there are top decorators. fig, ax = plt.subplots()
ax.set_title(f"Pad = -20", pad=-20)
ax.tick_params(axis="x",
bottom=True, top=True, labelbottom=True, labeltop=True)
plt.show() So here, the position has been moved to > 1.0 to make room for the axes, but the user has set pad to a negative number. I guess I think thats OK, its not too hard to get what they want...
|
OK. that breaks the fixes we put in if the title has a big overlap i.e. due to a subscript, which we all thought was a useful thing to do. #11502 I think the way forward is assume that if the user has axes.titlepad set then they don't want the auto-adjust behaviour. Folks who want the auto adjust can take what default we given time (autoadjust+6 points). That means changing the default for |
I think test failures are because the classic style sets |
e4f7360
to
2abccf8
Compare
I think this is ready for review now. Please see updated commentary above. |
b5a2198
to
d65fa16
Compare
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.
I'm not totally sure that pad implies no-auto positioning.
the lines and markers defaulting to whatever the first color in the color cycle was in the case of | ||
multiple plot calls. | ||
Previously setting the *ecolor* would turn off automatic color cycling for the plot, leading to the | ||
the lines and markers defaulting to whatever the first color in the color cycle was in the case of |
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.
Since you changed the line, there's a double 'the' that could be de-duplicated.
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.
Hmmmm, I didn't mean to change this line...
Since 3.0, Axes titles are automatically repositioned, primarily to avoid | ||
xlabels or xticks at the top of the axes. The user could turn this | ||
off using the optional *y* argument to `~.Axes.set_title`). However, that | ||
made it impossible to manually set the *pad* argument; Now *pad* defaults to |
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.
No need for a semicolon. Also, it should be "with y", no?
made it impossible to manually set the *pad* argument; Now *pad* defaults to | |
made it impossible to manually set the *pad* argument without also setting the *y* argument. Now *pad* defaults to |
lib/matplotlib/tests/test_axes.py
Outdated
# test that if we pad, then the title does not autopos | ||
|
||
fig, ax = plt.subplots() | ||
ax.set_title(f"Title 1", pad=-20) |
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.
Don't need the f-string.
@@ -19,7 +19,7 @@ def example_plot(ax, fontsize=12): | |||
ax.set_title('Title', fontsize=fontsize) | |||
|
|||
|
|||
@image_comparison(['tight_layout1']) | |||
@image_comparison(['tight_layout1'], tol=1.87) |
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.
Do you need to increase the tolerance, or can you not set the rcParam like the other test?
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.
No, its really strange - the whole figure is the same except for the blue line.
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.
This was the macOS failure with new Ghostscript? So this can now be rebased out.
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.
I don't think so, but maybe...
@QuLogic WRT the padding being set, I think the choices are: if ytop is the bounding box of the axes and all its decorators,
The user in the original issue clearly wanted 2 or 3. |
Automatic title repositioning is now suppressed if the title padding is set to a non-None value by the user (either in the pad kwarg, or the rcParam axes.titlepad)
lib/matplotlib/tests/test_axes.py
Outdated
@@ -5532,13 +5534,26 @@ def test_title_xticks_top(): | |||
def test_title_xticks_top_both(): | |||
# Test that title moves if xticks on top of axes. | |||
fig, ax = plt.subplots() | |||
# set to the new default (from 5, which will suppress title reposition) | |||
mpl.rcParams['axes.titlepad'] = None |
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.
Would it be better to switch these to use the new default style?
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.
I dunno, I admit to being thorough confused as to what style is default for the tests, versus which need to have the style specified somehow. It seems that all the tests are run classic
? But I don't know how to change a new test to use default if its not an image test.
I think the argument is that both The down side of pad -> no auto means we have no knob to adjust our auto behavior. Exploiting this degeneracy for "fine tuning" seems useful (if I am understanding this right). Given that this was changed in 3.0 and this is the first bug report about this, we may have picked up users who are now relying on the master branch behavior. I am +.5 on this, but that is a pretty soft position. Do we also auto-adjust the xlabels? |
@tacaswell good point: import matplotlib.pyplot as plt
fig, axs = plt.subplots(1, 2, constrained_layout=True)
axs[0].set_ylabel('Hi: labelpad=-4', labelpad=-4)
axs[1].set_ylabel('Hi: labelpad=+4', labelpad=4)
plt.show() returns |
So, based on that, I'd argue titlepad should be changed to be relative to whatever y the auhopositioning chooses (just like for the xlabel's pad) |
OK, looked into this some more: We could change this to make the baseline of the title relative to the top of the axes decorators plus the pad (of 6 pts). And then negative pads would be allowed. We didn't do this because of folks doing big math text in their title and complaining that it overhung the decorators. So before the change: BeforeAnd then we changed it to automatically do: After:But the way we do that is to see if the bbox for the title overhangs the axes and its decorators. If it does, we move it up. That is great, but if clearly is incompatible with a negative pad. Note that positive pads work fine. So, not sure what to do here. Tempted to keep the autopositioning behaviour and ask folks to just set y=0.0001 if they don't want the autopositioning. |
So, perhaps the solution is a new rcParam: |
So I think #17127 is a better way forward, but can re-open this if not... |
PR Summary
Closes #16805
When title autopositioning was put in #9498 it was overlooked that the user may manually specify the pad, and hence autopositioning should not take place.
Now yields
Similar behaviour is respected for
rcParam('axes.titlepad')
which now defaults toNone
Note: classic style (used in most tests) sets
axes.titlepad = 5
, so there were a couple of places intest_axes.py
that needed to manually set it to "None".There were two tests in
test_tightlayout.py
that failed with a pixel offset in the data line being drawn. I think this is a pixel snapping issue and elided it by setting the tolerance higher rather than trying to hunt this down. However, if folks really want it fixed, it probably means re-doing the image test (and probably making them work with a less fussy data set).PR Checklist