Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.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 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
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 (mostrecentcalllast):File"/home/travis/build/matplotlib/matplotlib/venv/lib/python3.5/site-packages/nose/case.py",line198,inrunTestself.test(*self.arg)File"/home/travis/build/matplotlib/matplotlib/lib/matplotlib/testing/decorators.py",line422,inbackend_switcherresult=func(*args,**kwargs)File"/home/travis/build/matplotlib/matplotlib/lib/matplotlib/tests/test_backend_pgf.py",line193,intest_bbox_inchestol=0)File"/home/travis/build/matplotlib/matplotlib/lib/matplotlib/tests/test_backend_pgf.py",line46,incompare_figureplt.savefig(actual,**savefig_kwargs)File"/home/travis/build/matplotlib/matplotlib/lib/matplotlib/pyplot.py",line686,insavefigres=fig.savefig(*args,**kwargs)File"/home/travis/build/matplotlib/matplotlib/lib/matplotlib/figure.py",line1666,insavefigself.canvas.print_figure(*args,**kwargs)File"/home/travis/build/matplotlib/matplotlib/lib/matplotlib/backend_bases.py",line2232,inprint_figure**kwargs)File"/home/travis/build/matplotlib/matplotlib/lib/matplotlib/backends/backend_pgf.py",line894,inprint_pdfself._print_pdf_to_fh(fh,*args,**kwargs)File"/home/travis/build/matplotlib/matplotlib/lib/matplotlib/backends/backend_pgf.py",line848,in_print_pdf_to_fhself.print_pgf(fname_pgf,*args,**kwargs)File"/home/travis/build/matplotlib/matplotlib/lib/matplotlib/backends/backend_pgf.py",line830,inprint_pgfself._print_pgf_to_fh(fh,*args,**kwargs)File"/home/travis/build/matplotlib/matplotlib/lib/matplotlib/backends/backend_pgf.py",line811,in_print_pgf_to_fhself.figure.draw(renderer)File"/home/travis/build/matplotlib/matplotlib/lib/matplotlib/artist.py",line63,indraw_wrapperdraw(artist,renderer,*args,**kwargs)File"/home/travis/build/matplotlib/matplotlib/lib/matplotlib/figure.py",line1262,indrawrenderer,self,dsu,self.suppressComposite)File"/home/travis/build/matplotlib/matplotlib/lib/matplotlib/image.py",line139,in_draw_list_compositing_imagesa.draw(renderer)File"/home/travis/build/matplotlib/matplotlib/lib/matplotlib/artist.py",line63,indraw_wrapperdraw(artist,renderer,*args,**kwargs)File"/home/travis/build/matplotlib/matplotlib/lib/matplotlib/axes/_base.py",line2355,indrawmimage._draw_list_compositing_images(renderer,self,dsu)File"/home/travis/build/matplotlib/matplotlib/lib/matplotlib/image.py",line139,in_draw_list_compositing_imagesa.draw(renderer)File"/home/travis/build/matplotlib/matplotlib/lib/matplotlib/artist.py",line63,indraw_wrapperdraw(artist,renderer,*args,**kwargs)File"/home/travis/build/matplotlib/matplotlib/lib/matplotlib/axis.py",line1133,indrawtick.draw(renderer)File"/home/travis/build/matplotlib/matplotlib/lib/matplotlib/artist.py",line63,indraw_wrapperdraw(artist,renderer,*args,**kwargs)File"/home/travis/build/matplotlib/matplotlib/lib/matplotlib/axis.py",line268,indrawself.label1.draw(renderer)File"/home/travis/build/matplotlib/matplotlib/lib/matplotlib/artist.py",line63,indraw_wrapperdraw(artist,renderer,*args,**kwargs)File"/home/travis/build/matplotlib/matplotlib/lib/matplotlib/text.py",line800,indrawismath=ismath,mtext=mtext)File"/home/travis/build/matplotlib/matplotlib/lib/matplotlib/backends/backend_pgf.py",line667,indraw_texttext_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' minusbaseline / 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
to697d8e1
CompareRebased. |
I'll need to rerun tests and squash -- stay tuned... |
c967de5
to7e4d26a
Comparec434aa1
tof9d695c
ComparePasssing 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 as5fb9d9e |
Fixes#2527. Align y-ticks without accounting for descenders