Implement PdfPages for backend pgf by maxnoe · Pull Request #10548 · matplotlib/matplotlib

maxnoe · 2018-02-20T15:26:37Z

PR Summary

This adds a PdfPages class for backend_pgf analogous to the one in backend_pdf to be able to use the pgf backend with multi page pdfs.

Looking forward to reviews

PR Checklist

Has Pytest style unit tests
Code is PEP 8 compliant
New features are documented, with examples if plot related
Documentation is sphinx and numpydoc compliant
Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

anntzer · 2018-02-20T15:58:55Z

+            for `'Creator'`, `'Producer'` and `'CreationDate'`. They
+            can be removed by setting them to `None`.
+        """
+        self.outputfile = filename


all these attributes should be private unless you explicitly have a reason to make some public.

anntzer · 2018-02-20T16:09:13Z

Would be nice to see how hard it is to implement a single MultiPage context for all relevant backends by adding the right methods to the RendererFoo classes. See e.g. https://github.com/anntzer/mplcairo/blob/master/lib/mplcairo/multipage.py as well.

But that may be too hard and a separate PR like this can work too.

jkseppan · 2018-02-21T07:23:11Z

Yes, a generic multipage interface would be nice, but I wouldn't make it a requirement for this PR. Similarly, I wouldn't worry about implementing pdf metadata in the first version, just note in the documentation that it's not yet supported. The LaTeX hyperref package is probably the way to go for implementing that.

jkseppan · 2018-02-21T07:29:55Z

We have tests that check that the output matches a baseline image. I think they currently exist only for the agg, pdf and svg backends, and I wouldn't really call them unit tests since they produce a complete output file and check the output. See https://github.com/matplotlib/matplotlib/tree/master/lib/matplotlib/testing for the infrastructure for the comparison tests. You could extend the pdf conversion to support multipage files, but that might get somewhat tricky.

anntzer · 2018-02-21T08:34:58Z

+        '_file',
+    )
+
+    def __init__(self, filename, keep_empty=True, metadata=None):


probably arguments after filename can be made keyword only

anntzer · 2018-02-21T08:35:32Z

+        """
+        self._outputfile = filename
+        self._n_figures = 0
+        self.keep_empty = keep_empty


would make private too

anntzer · 2018-02-21T08:36:07Z

+        Finalize this object, running LaTeX in a temporary directory
+        and moving the final pdf file to `filename`.
+        """
+        self._file.write(r'\end{document}'.encode('utf-8') + b'\n')


just use a single literal bytestring

maxnoe · 2018-02-21T08:54:59Z

I implemented your comments and setting metadata using hyperref

maxnoe · 2018-02-21T17:22:09Z

The Tests fail because the file I created in doc/users/next_whats_new is nowhere included. Where should it be included or what should I change?

jkseppan · 2018-02-21T17:48:13Z

Add something like this: 2124ac8

maxnoe · 2018-02-21T20:51:49Z

Now the test fails because the lualatex version on travis is ancient. Any idea what to do about that? Edit the requires_lualatex decorator so it also checks for a version?

jkseppan · 2018-02-22T19:14:36Z

Installing a newer TeX Live on Travis is a possibility but it sounds like it would really slow down the build. Does Appveyor have a sufficiently new luatex? If so, I think we could just check the version and skip the test if it is too old.

maxnoe · 2018-02-23T13:24:57Z

The failing Test Looks unrelated. Any idea what happened there? @jkseppan

jkseppan · 2018-02-23T15:38:59Z

Yes, the failure looks unrelated. I restarted the test to see if it happens again.

maxnoe · 2018-02-23T15:57:55Z

@jkseppan Ok thanks. The tests are passing now.

anntzer · 2018-02-23T19:14:36Z

+    return parse_lualatex_version(output.decode())
+
+
+def parse_lualatex_version(output):


that probably doesn't need to be public

anntzer · 2018-02-23T19:14:47Z

        warnings.warn('error getting fonts from fc-list', UserWarning)

+
+luatex_version_re = re.compile(


that definitely doesn't need to be public

anntzer · 2018-02-23T19:15:58Z

    return texsystem if texsystem in texsystem_options else "xelatex"


+def get_lualatex_version():


Note to other devs: would be nice to get #9571/#9639 merged first and integrate this version checking into that framework.

In the meantime, this function could(?) stay private.

jkseppan · 2018-02-24T08:04:59Z

@maxnoe Just in case it's not obvious: I think what @anntzer means by public vs private is that the indicated functions and variables could be named starting with an underscore (_luatex_version_re, etc) so that users of Matplotlib don't come to depend on them directly.

maxnoe · 2018-02-24T09:11:29Z

@anntzer I made all the version functions/attributes private

maxnoe · 2018-02-27T14:09:48Z

@anntzer @jkseppan I resolved the conflicts

maxnoe · 2018-02-27T18:01:41Z

@anntzer @jkseppan The tests are passing again

anntzer · 2018-02-27T19:51:54Z

+        latex_header = r"""\PassOptionsToPackage{{
+  {metadata}
+}}{{hyperref}}
+\RequirePackage{{hyperref}}


any reason why it's not a standard "usepackage[...]{hyperref}"? (perhaps, I dunno)

Because People might use hyperref in their rc params latex header and like this it avoids the "hyperref loaded with incompatible options" error

so that's equivalent to "if hyperref is already loaded by the preamble, then merge these options with those passed in the preamble"? (again, just asking)

Yes, and the user specified options take precedence.

anntzer · 2018-02-27T19:53:57Z

+            if self._n_figures == 0:
+                self._write_header(*figure.get_size_inches())
+            else:
+                if get_texcommand() == 'lualatex':


if get_texcommand == lualated and get_lualatex_version > ...?

anntzer · 2018-02-27T19:55:30Z

+                    np = r'\newpage\pdfpagewidth={}in\pdfpageheight={}in%'
+                self._file.write(np.format(
+                        *figure.get_size_inches()
+                    ).encode('utf-8') + b'\n'


just include the newline in the format-string above?
here it may make sense to just use a bytes literal and %-interpolation

anntzer · 2018-02-27T19:55:49Z

 needs_pdflatex = pytest.mark.skipif(not check_for('pdflatex'),
                                    reason='pdflatex + pgf is required')
+needs_lualatex = pytest.mark.skipif(not check_for('lualatex'),
+                                   reason='lualatex + pgf is required')


anntzer · 2018-02-27T19:57:22Z

 .. _pgf-rcfonts:

+
+Multi-Page PDF Files


add entries to doc/api/howto_faq (which mentions the old pdfpages) and examples/misc/multipage_pdf?

anntzer · 2018-02-28T09:10:24Z

Thanks for the PR!

anntzer · 2018-02-28T09:11:13Z

@meeseeksdev backport to v3.0

lumberbot-app · 2018-02-28T09:11:49Z

Something went wrong ... Please have a look at my logs.

maxnoe added 2 commits February 20, 2018 16:23

Implement PdfPages for backend pgf

1571c92

Cleanup tmpdir

1ce415c

anntzer reviewed Feb 20, 2018

View reviewed changes

maxnoe added 2 commits February 20, 2018 17:04

Make members private

455fc0c

Do not catch exception

7757bbc

Fix refactor leftover

b961af4

anntzer reviewed Feb 21, 2018

View reviewed changes

maxnoe added 3 commits February 21, 2018 09:50

Use hyperref to set pdf metadata

eb5a385

Make arguments kwargs only

7c7c135

Use single bytestring

8bfe11e

maxnoe added 2 commits February 21, 2018 10:39

Add docs for backend_pgf.PdfPages

4e55172

Add whats new for backend_pgf.PdfPages

3fa4701

maxnoe changed the title ~~WIP: Implement PdfPages for backend pgf~~ Implement PdfPages for backend pgf Feb 21, 2018

maxnoe added 3 commits February 21, 2018 11:41

pep8

e9128a2

Add test for metadata

0354d65

Support also lualatex in backend_pgf.PdfPages

f27d41d

Include next_whats_new

697f75b

Test for pagecount

e410bea

maxnoe added 2 commits February 23, 2018 09:58

PEP8

871cd5b

Install texlive-luatex on travis

66db38e

anntzer mentioned this pull request Feb 23, 2018

Remove some manual manipulation of whatsnew when releasing. #10573

Closed

6 tasks

anntzer reviewed Feb 23, 2018

View reviewed changes

Make lualatex version functions private

476ff78

Merge branch 'master' into pgf_pdf_pages

82cf8d0

Fix merge artifact

8cb4d51

anntzer reviewed Feb 27, 2018

View reviewed changes

maxnoe added 4 commits February 28, 2018 09:07

Implement review comments of @anntzer

afc26d8

Add .pytest_cache to gitignore

0f835a8

More docs

bc16483

Add comment in faq

05e7f77

anntzer merged commit a2db882 into matplotlib:master Feb 28, 2018

anntzer added this to the v3.0 milestone Feb 28, 2018

anntzer added the backend: pgf label Feb 28, 2018

maxnoe deleted the pgf_pdf_pages branch February 28, 2018 09:11

		return parse_lualatex_version(output.decode())


		def parse_lualatex_version(output):

		warnings.warn('error getting fonts from fc-list', UserWarning)


		luatex_version_re = re.compile(

		return texsystem if texsystem in texsystem_options else "xelatex"


		def get_lualatex_version():

Uh oh!

Uh oh!

Conversation

maxnoe commented Feb 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anntzer commented Feb 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jkseppan commented Feb 21, 2018

Uh oh!

jkseppan commented Feb 21, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maxnoe commented Feb 21, 2018

Uh oh!

maxnoe commented Feb 21, 2018

Uh oh!

jkseppan commented Feb 21, 2018

Uh oh!

maxnoe commented Feb 21, 2018

Uh oh!

jkseppan commented Feb 22, 2018

Uh oh!

maxnoe commented Feb 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jkseppan commented Feb 23, 2018

Uh oh!

maxnoe commented Feb 23, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkseppan commented Feb 24, 2018

Uh oh!

maxnoe commented Feb 24, 2018

Uh oh!

maxnoe commented Feb 27, 2018

Uh oh!

maxnoe commented Feb 27, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anntzer commented Feb 28, 2018

Uh oh!

anntzer commented Feb 28, 2018

Uh oh!

lumberbot-app Bot commented Feb 28, 2018

Uh oh!

maxnoe commented Feb 20, 2018 •

edited

Loading

anntzer commented Feb 20, 2018 •

edited

Loading

maxnoe commented Feb 23, 2018 •

edited

Loading