Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
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 forall titles, not just for the ones that would overlap: importmatplotlib.pyplotaspltfig,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 offsetif 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 in88ea09b
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 in88ea09b
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 shouldalways 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 shouldalways 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. |
jklymak left a comment• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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:
importmatplotlib.pyplotaspltfig,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 spillingwell into the axes above.
In addition, this saves an extra get_tightbbox if its not needed, which can be expensive.
Uh oh!
There was an error while loading.Please reload this page.
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. |
jklymak commentedJan 5, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I guess this needs an API change note; put in draft until its added, definitely feel free to pop back.... |
7182602
tocbec785
CompareUh oh!
There was an error while loading.Please reload this page.
Move any title above the y axis offset text it would overlap with theoffset. If multiple titles are present, they are vertically aligned tothe highest one.
Uh oh!
There was an error while loading.Please reload this page.
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).