search

NNPDF / reportengine

A framework for declarative data analysis
https://data.nnpdf.science/validphys-docs/guide.html
GNU General Public License v2.0
1 stars 2 forks source link

minor change as a global workaround for NNPDF/nnpdf#363 #17

Closed wilsonmr closed 5 years ago

wilsonmr commented 5 years ago

See matplotlib/matplotlib/#13276

The 3.0.2 fix of matplotlib/matplotlib#12648 (by PR https://github.com/matplotlib/matplotlib/pull/12651) appears to only affect plt.tight_layout(), not the kwarg bbox_inches='tight' (see matplotlib/matplotlib#13276). This minor change means that users who have MPL 3.0.2 installed shouldn't encounter issues caused by annotations with NaN position.

It will also be accompanied by a PR in NNPDF/nnpdf which tries to avoid creating annotations with NaN positions

Zaharid commented 5 years ago

Does this produce observable differences?

wilsonmr commented 5 years ago

I will check tomorrow

wilsonmr commented 5 years ago

So the actual figure appears to have the same border to plot ratio, the main difference is .tight_layout() seems to enlarge the plot to make the layout tight, whereas the bbox_inches='tight' shrinks the image size to be tight around the plot. Finally using both together makes the size of the image slightly smaller than what I guess is the default size. I suppose it depends if when the PNGs are embedded into the reports if they are resized to be a fixed width? If not then you might notice that the plots get slightly bigger, although their appearance should remain largely unchanged. If I plot them instead as PDFs then basically the bbox_inches='tight' has lower resolution

Zaharid commented 5 years ago

I guess there is some value in controlling the actual size in the client code when creating a figure. That said, it is probably better to get this to the point where it does not lead to crashes.

Zaharid commented 5 years ago

So this is to say that I am inclined to merge this, and maybe revert it once the bug is fixed upstream and we can require newer version.

wilsonmr commented 5 years ago

As you wish. Whilst doing NNPDF/nnpdf#364 I grepped annotate and fixed the cases addressed the ones which hadn't already been touched so possibly this is overkill.

But having said that it should help if there were any other cases unconsidered, and like you say it can be reverted later