GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Improve PDF metadata support in PGF by QuLogic · Pull Request #17233 · matplotlib/matplotlib

Conversation

@QuLogic

@QuLogic QuLogic commented Apr 24, 2020

Copy link
Copy Markdown
Member

PR Summary

The PGF backend only supported PDF metadata using the multi-page PdfPages class; this adds support for adding metadata to single-figure saving directly via savefig.

Additionally, refactor the metadata support from the PDF side into a common function to setup and verify metadata that can be used in the PGF side as well. This ensures that the same set of keys is allowed, which weren't before.

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@QuLogic QuLogic added backend: pgf backend: pdf labels Apr 24, 2020
@QuLogic QuLogic added this to the v3.3.0 milestone Apr 24, 2020
@QuLogic QuLogic mentioned this pull request Apr 24, 2020
Warn on invalid savefig keyword arguments #17223
Merged
1 of 6 tasks
@QuLogic
QuLogic force-pushed the pgf-pdf-metadata branch from 9152fc8 to cbd4ca3 Compare May 5, 2020 03:35
@tacaswell

tacaswell commented May 23, 2020

Copy link
Copy Markdown
Member

👍 In principle, but I think that this should get some through tests while we are thinking about it.

Do we want to push this to 3.4 or try to squeeze it in for 3.3?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
anntzer
anntzer approved these changes May 23, 2020
View reviewed changes

@anntzer anntzer left a comment

Copy link
Copy Markdown
Contributor

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.

looks reasonable.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@QuLogic QuLogic added the status: needs rebase label May 25, 2020
@QuLogic
QuLogic force-pushed the pgf-pdf-metadata branch from cbd4ca3 to 1bf86d3 Compare May 27, 2020 00:05
@QuLogic QuLogic removed the status: needs rebase label May 27, 2020
@QuLogic

QuLogic commented May 27, 2020

Copy link
Copy Markdown
Member Author

This is difficult to test, because unlike the builtin PDF export, the PGF backend uses latex, which has no pdf.compression setting, and all the metadata is compressed. You can check in evince that the metadata is set from the test though.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@QuLogic QuLogic mentioned this pull request May 28, 2020
Reuse png metadata handling of imsave() in FigureCanvasAgg.print_png(). #15435
Merged
6 tasks
@QuLogic
QuLogic force-pushed the pgf-pdf-metadata branch from 1bf86d3 to 911e54d Compare June 1, 2020 20:37
@anntzer

anntzer commented Jun 1, 2020

Copy link
Copy Markdown
Contributor

I guess you could possibly use pdfinfo (from poppler) or mutool (from mupdf) as optional test dependency to extract the metadata?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@QuLogic

QuLogic commented Jun 2, 2020

Copy link
Copy Markdown
Member Author

It doesn't print Trapped, but it looks like pdfinfo would work for the others. There might be a bug in one or the other, because the printed date is wrong...
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@QuLogic

QuLogic commented Jun 2, 2020

Copy link
Copy Markdown
Member Author

Also, another possibility is pikepdf, which we could use from Python.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
QuLogic added 7 commits June 2, 2020 01:23
@QuLogic
pgf: Pass PDF metadata via pdfinfo option.
c4613f8 
This takes keys that match the PDF specification instead of lower-case,
'pdf'-prefixed keys, so is a bit simpler. Also, deprecate ability to
accept key case-insensitively, as that is not done in the PDF backend.
@QuLogic
pdf/pgf: Move PDF metadata checks to a common function.
1127bbd 
This ensures that the same default values are produced, and more
importantly ensures that the same keys are accepted. Previously, PGF
only allowed a subset of keys.
@QuLogic
pdf: Don't require Trapped to be a Name instance.
31e462b 
It's something internal to the PDF backend, and there's no need to
require the metadata to use that type instead of a plain string.
@QuLogic
Implement PDF metadata on PGF backend.
65ac78f 
This is supported through the multipage PdfPages, but not direct output
of a single figure via PGF.
@QuLogic
Add What's New entry about previous metadata changes.
ae098ea
@QuLogic
Combine PdfPages tests.
aa0fa1a 
They're basically the same, but with different `pgf.texsystem` settings.
@QuLogic
Add tests for PDF metadata saving using pikepdf.
cf4a117
@QuLogic
QuLogic force-pushed the pgf-pdf-metadata branch from 911e54d to cf4a117 Compare June 2, 2020 22:39
@QuLogic

QuLogic commented Jun 2, 2020

Copy link
Copy Markdown
Member Author

That was a good idea, because I found a few bugs in the PGF side, as hyperref seems to have some differing requirements on the format for things.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@tacaswell
tacaswell merged commit ecdfa65 into matplotlib:master Jun 8, 2020
@QuLogic
QuLogic deleted the pgf-pdf-metadata branch June 8, 2020 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@anntzer anntzer anntzer approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

backend: pdf backend: pgf

Projects

None yet

Milestone

v3.3.0

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@QuLogic @tacaswell @anntzer
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.