-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
FIX: Autoposition title when yaxis has offset #22063
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
Hi @StefRe , I came across this PR, thank you. Previously I just manually adjusted the position of the title. I think it will be great to also consider the x position of fig, ax = plt.subplots()
ax.plot([1e8, 2e8])
ax.set_title('Left', loc='left')
ax.set_title('Center')
ax.set_title('Right', loc='right')
plt.show() |
@yuanx749 Thanks for your feedback. I don't think, however, we should strive to cover all possible cases: moving the title up to not overlap with top ticks (which is current behavior) also sets the title position for all titles, not just for the ones that would overlap: import matplotlib.pyplot as plt
fig, ax = plt.subplots(layout='constrained')
ax.plot([1e8, 2e8], [1, 2])
ax.set_title('Left', loc='left')
ax.set_title('Center')
ax.set_title('Right', loc='right')
ax.xaxis.tick_top()
plt.show() |
I'm 50/50 on this. I'm not sure we want titles in the centre to be so high so maybe only do this if the left title is not empty? |
If we have multiple titles as in the example (which is, admittedly, seldom in practice), the different hights of the different titles would look awkward. |
I agree. I just meant that we only move all three of the left title is occupied. I guess a counter argument is that some centred titles can be very long, but I think most of the time folks are going to complain about all the wasted vertical space. |
Maybe we can consider this as a separate issue and only change one thing at a time, just to not overcomplicate things:
|
I guess I'm not being clear. My proposal is you only change the y position to account for the y offset if there is a string in the left title. I'm not proposing 2 or more separate positions. |
@jklymak I think I understood you correctly - the only thing I wanted to say is that if we move the left title only (to not overlap with an y axis offset text), then we should also change the existing code so that center and left titles are moved up just above the top x tick labels, not above the x offset text (in case there is an x offset). Right now all titles are treated the same way: matplotlib/lib/matplotlib/axes/_base.py Lines 2966 to 2968 in 88ea09b
If we want to treat the left title separately this logic must be changed a bit. Further, all titles are currently lined up at one level: matplotlib/lib/matplotlib/axes/_base.py Lines 3008 to 3012 in 88ea09b
So this will also be needed to addressed (in case there are multiple titles). This is why I suggested to deal with these things in a separate issue "Update title positions per title to move them just the necessary amount", if this is really needed. |
@StefRe, I am not sure you did understand correctly, or I'm not understanding you. I am agreeing with you that the vertical level of all three titles should always be the same. And in general I am agreeing with fixing the overlap between a left-side title and the y-axis offset text. However, I disagree that we should always displace all the titles if there is y-axis offset text. That is a breaking change that will affect many people, adding undesirable new vertical space to 99.9% of the users who just have a centered title. You mention the precedent of the x-axis being taken into account, in particular when it is at the top of the axes. That is, in fact, the whole point of the automatic repositioning feature in the first place. Having the extra space was deemed useful because the top of the axes was already full of x-tick labels, and usually the xlabel itself. I don't think that applies to an offset text on the left side and most centred titles. Just to re-iterate, I think your PR should go in as-is, except the logic should only be called if the left title string is non-empty. (Or the right title string, if the y axis is on the right, I suppose). If that is satisfied, I agree all three titles should move up together. But I don't think we should do this if the left title string is empty. |
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.
My suggestion is above. On the code:
import matplotlib.pyplot as plt
fig, axs = plt.subplots(2, 1, constrained_layout=True)
ax = axs[0]
ax.plot([1e8, 2e8])
ax.set_title('Left', loc='left')
ax.set_title('Center')
ax.set_title('Right', loc='right')
ax = axs[1]
ax.plot([1e8, 2e8])
ax.set_title('Center')
ax.set_title('Right', loc='right')
plt.show()
With the suggested change it yields:
The suggested change is also not a breaking change, except if the left title is populated, in which case it is a desired change.
If you don't use constrained_layout it is even worse with the new title position spilling well into the axes above.
In addition, this saves an extra get_tightbbox if its not needed, which can be expensive.
862b4a1
to
da7a16a
Compare
Updated version which only moves the title if it indeed would overlap and aligns them if there are multiple titles:
Just for the records: this PR doesn't change title movement in case of top x axis (as discussed above):
Demo code:
|
Hm, I'm a bit at a loss: there are 7 failing tests
for Python 3.8 + 3.9 on Ubuntu (they pass on 3.7 and 3.10 as well as for all versions on Windows and macOS) and I don't see how the failures are related to this PR:
I can't debug as I only have a Python 3.10.1 Arch and a Python 3.8 Windows computer and all tests pass here. Do you have any idea or hint how to proceed from here? |
We are not sure what is causing the tests to fail, but they are failing on doc PRs so something has gone wrong in our library chain. |
I guess this needs an API change note; put in draft until its added, definitely feel free to pop back.... |
7182602
to
cbec785
Compare
Move any title above the y axis offset text it would overlap with the offset. If multiple titles are present, they are vertically aligned to the highest one.
cbec785
to
cfabe79
Compare
PR Summary
Move the title above the offset text if present.
OLD:


NEW:
Closes #22062.
PR Checklist
Tests and Styling
pytest
passes).flake8-docstrings
and runflake8 --docstring-convention=all
).Documentation
doc/users/next_whats_new/
(follow instructions in README.rst there).doc/api/next_api_changes/
(follow instructions in README.rst there).