Skip to content
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

Add per-page pdf notes in PdfFile and PdfPages. #3994

Merged
merged 4 commits into from
Feb 8, 2015

Conversation

ulido
Copy link

@ulido ulido commented Jan 12, 2015

I was looking for the possibility to add per-figure metadata to a PdfPages output file. This patch enables this and adds a new method attach_note to the PdfPages class. This methods needs to be called with an arbitrary piece of text that is added as a note to the following page (hence attach_note needs to be called before savefig). The logic for creating the underlying pdf annotation object sits in the newNote method of the PdfFile class. The note is visible in any pdf reader that is able to display a list of annotations.

@tacaswell
Copy link
Member

Can you please fix the pep8 violations?

======================================================================
FAIL: matplotlib.tests.test_coding_standards.test_pep8_conformance_installed_files
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/matplotlib-1.5.dev1-py2.7-linux-x86_64.egg/matplotlib/tests/test_coding_standards.py", line 255, in test_pep8_conformance_installed_files
    expected_bad_files=expected_bad_files)
  File "/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/matplotlib-1.5.dev1-py2.7-linux-x86_64.egg/matplotlib/tests/test_coding_standards.py", line 140, in assert_pep8_conformance
    assert_equal(result.total_errors, 0, msg)
AssertionError: Found code syntax errors (and warnings):
/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/matplotlib-1.5.dev1-py2.7-linux-x86_64.egg/matplotlib/backends/backend_pdf.py:483:34: E261 at least two spaces before inline comment
/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/matplotlib-1.5.dev1-py2.7-linux-x86_64.egg/matplotlib/backends/backend_pdf.py:529:45: E251 unexpected spaces around keyword / parameter equals
/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/matplotlib-1.5.dev1-py2.7-linux-x86_64.egg/matplotlib/backends/backend_pdf.py:529:47: E251 unexpected spaces around keyword / parameter equals
/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/matplotlib-1.5.dev1-py2.7-linux-x86_64.egg/matplotlib/backends/backend_pdf.py:2472:1: E302 expected 2 blank lines, found 1

@tacaswell
Copy link
Member

Look reasonable to me.

This is probably also worth a mention in https://github.com/matplotlib/matplotlib/tree/master/doc/users/whats_new and can you find and update the pdf pages example file? New features don't count unless they are documented ;)

attn @jkseppan

Ulrich Dobramysl added 3 commits January 13, 2015 07:26
@ulido
Copy link
Author

ulido commented Jan 13, 2015

Thanks for your comments!

@tacaswell - I corrected the PEP8 violations, sorry for that. The whats_new section contains an entry for this feature now, and I added a simple line to the multipage_pdf.py example.

@jkseppan - The default position is off-page, so that the note icon doesn't appear in the figure itself. The reasoning being that the metadata should only appear in the list of annotations of a pdf viewer, not in the figure itself.

You are right, it might make sense to make it similar to the draw_* functions. I wasn't coming from that angle as I didn't want to draw anything on the figure itself though, just add per-page metadata text. I can change the code to be more similar to the draw_* functions if that sounds better to you though.

@tacaswell tacaswell added this to the v1.5.x milestone Jan 19, 2015
@tacaswell
Copy link
Member

@jkseppan If you are happy with this as-is please merge it, other wise can you be in charge of the changes?

@jkseppan jkseppan merged commit f65281a into matplotlib:master Feb 8, 2015
jkseppan added a commit that referenced this pull request Feb 8, 2015
This is ulido:pdf-annotations with a small documentation addition
to advertise it a little bit better.

Closes #3994
@ulido ulido deleted the pdf-annotations branch February 9, 2015 07:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants