-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Tick vertical alignment #6200
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
Tick vertical alignment #6200
Conversation
Seems to have broken all of the pgf tests. That makes me a bit worried about other backends that we do not control.... |
Sorry -- I have a new box and I think I don't have all the pgf reqs installed yet. I can probably regenerate those. As for other backends we don't control, I think we'll just be seeing better tick text alignment in all of them -- this approach is delibrately rather narrow and shouldn't affect other kinds of text. But, yes, we will be breaking those other backends image comparison tests, if any. |
======================================================================
ERROR: matplotlib.tests.test_backend_pgf.test_bbox_inches
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/travis/build/matplotlib/matplotlib/venv/lib/python3.5/site-packages/nose/case.py", line 198, in runTest
self.test(*self.arg)
File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/testing/decorators.py", line 422, in backend_switcher
result = func(*args, **kwargs)
File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/tests/test_backend_pgf.py", line 193, in test_bbox_inches
tol=0)
File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/tests/test_backend_pgf.py", line 46, in compare_figure
plt.savefig(actual, **savefig_kwargs)
File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/pyplot.py", line 686, in savefig
res = fig.savefig(*args, **kwargs)
File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/figure.py", line 1666, in savefig
self.canvas.print_figure(*args, **kwargs)
File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/backend_bases.py", line 2232, in print_figure
**kwargs)
File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/backends/backend_pgf.py", line 894, in print_pdf
self._print_pdf_to_fh(fh, *args, **kwargs)
File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/backends/backend_pgf.py", line 848, in _print_pdf_to_fh
self.print_pgf(fname_pgf, *args, **kwargs)
File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/backends/backend_pgf.py", line 830, in print_pgf
self._print_pgf_to_fh(fh, *args, **kwargs)
File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/backends/backend_pgf.py", line 811, in _print_pgf_to_fh
self.figure.draw(renderer)
File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/artist.py", line 63, in draw_wrapper
draw(artist, renderer, *args, **kwargs)
File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/figure.py", line 1262, in draw
renderer, self, dsu, self.suppressComposite)
File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/image.py", line 139, in _draw_list_compositing_images
a.draw(renderer)
File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/artist.py", line 63, in draw_wrapper
draw(artist, renderer, *args, **kwargs)
File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/axes/_base.py", line 2355, in draw
mimage._draw_list_compositing_images(renderer, self, dsu)
File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/image.py", line 139, in _draw_list_compositing_images
a.draw(renderer)
File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/artist.py", line 63, in draw_wrapper
draw(artist, renderer, *args, **kwargs)
File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/axis.py", line 1133, in draw
tick.draw(renderer)
File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/artist.py", line 63, in draw_wrapper
draw(artist, renderer, *args, **kwargs)
File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/axis.py", line 268, in draw
self.label1.draw(renderer)
File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/artist.py", line 63, in draw_wrapper
draw(artist, renderer, *args, **kwargs)
File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/text.py", line 800, in draw
ismath=ismath, mtext=mtext)
File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/backends/backend_pgf.py", line 667, in draw_text
text_args.append(valign[mtext.get_va()])
KeyError: 'center_baseline' This looks like the render is breaking. |
Oh, and I should have read the error message first. It seems that PGF, unlike all other backends in the core, handles the text alignment itself -- or, more specifically, delgates it to the TeX engine. That seems like a corner case to me that most third-party backends won't face, but point taken that it could break them. However, we are bumping the major version number for the first time in years, if not now, when? :) |
fair point. |
I am 👍 on this, but given how many tests it touches someone else should agree as well. |
@@ -436,6 +436,8 @@ def _get_layout(self, renderer): | |||
offsety = (ymin + height) | |||
elif valign == 'baseline': | |||
offsety = (ymin + height) - baseline | |||
elif valign == 'center_baseline': | |||
offsety = (ymin + (height + (height - baseline)) / 2.0) |
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.
In other words, ymin + height - baseline / 2
. This is the same as 'top' minus baseline / 2
which makes sense, since baseline is the height starting from the baseline rather than the descender. Right?
I think it is OK. The relevant part of the text code was already not so easy to understand, so I made a couple of suggestions for comments and simplification. I don't understand why the anchored rotation case is handled as it is. |
I've hopefully addressed @efiring's comments. |
@mdboom Now you get to rebase this one 👿 |
7293734
to
697d8e1
Compare
Rebased. |
I'll need to rerun tests and squash -- stay tuned... |
c967de5
to
7e4d26a
Compare
c434aa1
to
f9d695c
Compare
Passsing the tests! Let's merge this before I have to rebase and rebuild all the tests again :) |
I'm backporting this to 2.x now, but there's some conflicts, so it might take a while... |
Tick vertical alignment
Backported to 2.x as 5fb9d9e |
Fixes #2527. Align y-ticks without accounting for descenders