Don't fallback to agg in tight_layout.get_renderer. #15221
Conversation
| try: | ||
| print_method(io.BytesIO()) |
There was a problem hiding this comment.
dpi was many other things than figure.dpi in my quick skim of the code below, so is it really OK to use figure.dpi here? I'll block because I think this should be answered, but anyone should clear if this is really not a problem. I haven't looked carefully yet.
There was a problem hiding this comment.
If _get_renderer is called from print_figure(), then figure.dpi has already been set to match the requested savefig dpi (cbook._setattr_cm(self.figure, dpi=dpi)), so setting it a second time is a noop. If called from tight_layout.get_renderer, OTOH, we do need to pass in the figure dpi -- otherwise it's savefig.dpi which gets used, which is not correct when we want to tight-layout something not for the purpose of saving it.
b4a11dd to
fe8560b
Compare
|
I don't think this rebase went correctly... |
|
fixed the rebase |
|
As an aside, this closes #1852, I think. |
|
This seems to break the output from #9158. |
|
Ah no wait, it's just because this branch is outdated and EPS was broken for some time, but fixed on |
|
I'm a bit confused: what's broken? (if anything?) |
|
Nothing. The bug is fixed on |
|
Codecov is not thrilled with this. Is it humanly possible to make tests for this? |
tight_layout.get_renderer currently falls back to agg if it is unable to
get a renderer for the current canvas, but we actually have a way to
coerce the canvas to produce a renderer, via
backend_bases._get_renderer.
This renderer gets used by the tight_layout machinery to estimate text
size. Falling back to Agg would produce bad results in the (admittedly
rare) cases where text size estimation yields very different results
depending on the renderer -- the easiest way to trigger this is to use
a "thin" font, e.g. Tex Gyre Chorus, while using the pdf backend with
the "pdf.use14corefonts" rcParam set to True, which effectively forces
Helvetica. e.g.
```
from pylab import *
matplotlib.use("pdf")
rcdefaults()
rcParams.update({"pdf.use14corefonts": True, "font.family": "TeX Gyre Chorus"})
plot()
margins(0)
tight_layout(0)
savefig("/tmp/test.pdf")
```
would crop the tick labels before this PR.
(The real point is not to fix an obscure edge case, but actually to have
fewer places that fall back to Agg as "favorited" backend.)
|
added a test. |
tight_layout.get_renderer currently falls back to agg if it is unable to
get a renderer for the current canvas, but we actually have a way to
coerce the canvas to produce a renderer, via
backend_bases._get_renderer.
This renderer gets used by the tight_layout machinery to estimate text
size. Falling back to Agg would produce bad results in the (admittedly
rare) cases where text size estimation yields very different results
depending on the renderer -- the easiest way to trigger this is to use
a "thin" font, e.g. Tex Gyre Chorus, while using the pdf backend with
the "pdf.use14corefonts" rcParam set to True, which effectively forces
Helvetica. e.g.
would crop the tick labels before this PR.
(The real point is not to fix an obscure edge case, but actually to have
fewer places that fall back to Agg as "favorited" backend.)
PR Summary
PR Checklist