Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Include outward ticks in bounding box#5683
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
This needs a test specifically of tight layout on outward ticks, but I thought I'd put this up for comment early anyway... |
I think you meant this replaces#5502? |
Oops. You're right. |
Well, I don't see anything particularly wrong with it, but are there no test images that use outward ticks? Or just none with a tight bounding box? |
A test has been added |
| x_pad = self.xaxis.get_tick_padding() | ||
| y_pad = self.yaxis.get_tick_padding() | ||
| return mtransforms.Bbox([[bbox.x0 - x_pad, bbox.y0 - y_pad], | ||
| [bbox.x1 + x_pad, bbox.y1 + y_pad]]) |
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.
Is this going to work for polar axes? I would also double-check that this doesn't utterly bork for axes3d (not that I'd expect it to work great there, just to not crash or something).
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.
polar axes don't have ticks -- but it does appear that this needs to be updated so it's aware of when the ticks are present or not.
It doesn't seem to crash for axes3d, though as you say, it doesn't really do the right thin other than add a bit more space.
image tests that have just been added are failing |
I don't think the test is really all that useful. tight_layout is for ensuring that subplots don't run into each other. So, a good test would be to have a grid of subplots, perhaps each with different tick directions? |
83e8a00 to14190e4Compare@WeatherGod: I've updated the test as you helpfully suggested. |
| Get the length of the tick outside of the axes. | ||
| """ | ||
| tickdir = self._tickdir | ||
| if tickdir == 'in': |
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.
bit of a bike shed, better to do this with a dictionary?
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.
Sure.
I've addressed@tacaswell's comments, and added minor ticks to the test to catch the pathological case he mentions. |
Also updated to handle the case where there are no ticks, as suggested by@WeatherGod |
Include outward ticks in bounding box
backported as2a6bb26 |
Include outward ticks in bounding box
Include outward ticks in bounding box
Backport to v2.x was actually via8ac5b4b. |
Replaces#5502.