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

PDF figures as links from png or svg figures #4130

Closed
wants to merge 4 commits into from
Closed

PDF figures as links from png or svg figures #4130

wants to merge 4 commits into from

Conversation

jsw-fnal
Copy link

I've implemented a feature that I've wanted for a while. When a figure is displayed, the img is a link to a PDF version of the figure. This follows the convention for webpages in my collaboration, where a figure should be displayed on the webpage as a raster, but a vector (eps or pdf) needs to be available for people to use in LaTeX.

To do this, I made four changes. First, I added a PDFFormatter class to formatters.py, and registered it as a formatter. Then, I changed pylabtools.py to always generate PDF output, whether figure_format is "png", "svg", or "retina". I think this part still needs some work, as it is currently not configurable. Then, I made jsonutil.py detect and encode PDF data in base64. Finally, I added the necessary javascript to outputarea.js to wrap the anchor tag around the img tag.

I have not tested how this interacts with nbconvert, because my system does not have Haskell available. I have to convince the sysadmins that I really need it before I can try to set up pandoc. So I may need some assistance in that regard.

I hope that this new feature is helpful and can eventually make it into ipython.
Best regards,
Jon

of figure_format.  This can probably be done better, since the way I've
done it, it is not configurable at all.
… href and with the download attribute set, when application/pdf data is available from JSON
@minrk
Copy link
Member

minrk commented Aug 29, 2013

Thanks for re-issuing, now we can start reviewing in earnest. A few points:

  1. The PDF output should not be on by default.
  2. The PDF UI should probably not be conditional on there also being an image (for instance, it technically makes sense for a user to only have PDF figures if their goal is just nbconvert latex output, rather than needing interactive inline figures.

I think it's appropriate for this initial PR to do a little bit less:

  1. define the PDF formatter
  2. tell jsonutil about PDF as a binary format
  3. provide an API function for enabling PDF output for matplotlib figures

We can save the js UI for PDFs for later work.

@jakobgager
Copy link
Contributor

I agree with @minrk the pdf option is nice, however it should be configurable and disabled by default. Otherwise, the
notebooks might become huge in some cases.

@minrk
Copy link
Member

minrk commented Aug 29, 2013

"however" sounds like you disagree, but my very first point was that PDF should not be on by default, so I think we are on the same page.

@jakobgager
Copy link
Contributor

Sorry for my ambiguous english 😳
I just wanted to point out that the feature itself sounds really useful in many situations (I guess that's your opinion) but I'm against enabling it by default (that's your point too).

@jsw-fnal
Copy link
Author

Yes, I agree that it should not be on by default. I think that figure_format is the right place to configure this, but currently that is set up so that only one figure type can be chosen at a time. I, for one, have a definite use-case in mind where I want two figure types turned on at once (png and pdf). Should we allow only one of png/retina/svg and have pdf turned on and off separately (could get complicated quickly if more figure types get added, for instance eps), or should we allow any combination of figure types to be turned on (simpler, the potential harm I see is larger notebooks)?

If the latter, is there already an extant config mechanism in place for turning on a set of options? I'm imagining something like

%config InlineBackend.figure_format = {'svg', 'pdf', 'eps'}

I'm also happy to remove the JS stuff for now -- I think my changes are somewhat kludgy as is (doesn't work for SVG+PDF, for example), so will need more careful work before they are ready to be incorporated. I'll continue using what I have now, though. It works perfectly for me and my use-case. :-)

@Carreau
Copy link
Member

Carreau commented Aug 29, 2013

@jsw-fnal note that we want to have a general way of sending any mimetype to the browser. We are moving away from special-casing everything, so we will be happy to get you to contribute.

Personnaly I would change figure_format to InlineBackend.figure_format**s** that could be a set/list of representations to send to the frontend. That what IJulia does.

Any step toward this would be great I think.

As for the javascript thing It might be too specific also. A way of dynamically switching representation might be great to.

@ellisonbg
Copy link
Member

What do we want to do with this PR? If we plan on work continuing, can we create a todo list for what needs to be done?

@jsw-fnal
Copy link
Author

@ellisonbg, I don't know that I have time right now to refactor the system of sending mimetypes to the browser, as @Carreau says. That would require considerably more comprehension of the existing code than I have right now. If you're willing to wait perhaps several months, I'm happy to spend the time, as I have it, to try to figure this out.

Regarding the JS, I'm no UX expert, so perhaps it is best to let someone else tackle that.

I'll take a stab at a todo list:

  1. Remove changes to JS
  2. Turn figure_format into figure_formats -- a set/list of representations to send to the frontend
  3. Develop alternate JS for displaying whatever set of representations are provided
  4. Refactor mimetypes system to be more general

I think that, if 1 and 2 are done, perhaps you could merge this, and leave 3 and 4 for future work?

@minrk
Copy link
Member

minrk commented Sep 20, 2013

I would actually skip 2. as well - there is a high cost to changing configurable names, and I don't think adding an s is sufficiently beneficial to warrant breakage.

I also think the general addition of mime types is out of scope for this PR, and to be done by someone with more intimate knowledge of the affected pieces. I think the list I made above should be sufficient:

  • define the PDF formatter
  • tell jsonutil about PDF as a binary format
  • provide an API function for enabling PDF output for matplotlib figures (off by default, maybe implemented via figure_format)

@ellisonbg
Copy link
Member

@jsw-fnal any progress on this PR, specifically the tasks @minrk outlined above?

@jsw-fnal
Copy link
Author

Unfortunately, no. I'm in the middle of a push to get a result out and I'm changing jobs at the end of the year, so it may be a little while before I get back to this. Should we just close the PR for now? I can re-open it I guess when I can get back to this.

On the plus side, we got this paper accepted to Phys Rev Lett last week. It cites you guys :)

@damianavila
Copy link
Member

As @jsw-fnal will be busy, what we want to do with issue? There is somebody interested on keeping forward? If not, I think we can close it and let @jsw-fnal re-opened when s/he has sometime available...

@damianavila
Copy link
Member

As @jsw-fnal will be busy, what we want to do with issue? There is somebody interested on keeping forward? If not, I think we can close it and let @jsw-fnal re-opened when s/he has sometime available...

Thoughts?

@damianavila
Copy link
Member

So... what do we do with this one?

@ghost ghost assigned ellisonbg Jan 13, 2014
@ellisonbg
Copy link
Member

We talked about this today at the dev meeting and decided the following:

  • pdf output in the matplotlib inline backend should be off by default. In a separate PR, we would like to follow up with a general API with the backend that allows multiple output formats to be enabled.
  • OutputArea.append_pdf should just display a "Download PDF" button/link and not try to wrap png/jpeg/svg output. IOW the handling of these different output types should be orthogonal. In this PR, the "Download PDF" button will only show if there are no other output types. In the future, we will add UI support for changing which format is being viewed.

@ellisonbg
Copy link
Member

Also needs a rebase.

@ellisonbg
Copy link
Member

@jsw-fnal do you have plans on getting to this soon. It would be great to have this in for the 2.0 release which is going to be in the next few weeks. If you don't think you will find time to work on it, just let us know and we can probably take over.

@jsw-fnal
Copy link
Author

Sadly, I think I will not be able to do this on that timescale.
Hopefully, I'll be able to make another stab at an IPython development
task sometime in the future. It's a wonderful piece of software that I
use on a near-daily basis.
Jon

On 01/27/2014 04:19 PM, Brian E. Granger wrote:

@jsw-fnal https://github.com/jsw-fnal do you have plans on getting to
this soon. It would be great to have this in for the 2.0 release which
is going to be in the next few weeks. If you don't think you will find
time to work on it, just let us know and we can probably take over.


Reply to this email directly or view it on GitHub
#4130 (comment).

@ellisonbg
Copy link
Member

OK I will take over for you - thanks for bringing it this far!

On Mon, Jan 27, 2014 at 2:21 PM, jsw-fnal notifications@github.com wrote:

Sadly, I think I will not be able to do this on that timescale.
Hopefully, I'll be able to make another stab at an IPython development
task sometime in the future. It's a wonderful piece of software that I
use on a near-daily basis.
Jon

On 01/27/2014 04:19 PM, Brian E. Granger wrote:

@jsw-fnal https://github.com/jsw-fnal do you have plans on getting to
this soon. It would be great to have this in for the 2.0 release which
is going to be in the next few weeks. If you don't think you will find
time to work on it, just let us know and we can probably take over.


Reply to this email directly or view it on GitHub
#4130 (comment).


Reply to this email directly or view it on GitHubhttps://github.com//pull/4130#issuecomment-33430171
.

Brian E. Granger
Cal Poly State University, San Luis Obispo
bgranger@calpoly.edu and ellisonbg@gmail.com

@ellisonbg
Copy link
Member

Ongoing work is here #4921

@ellisonbg ellisonbg closed this Jan 29, 2014
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

6 participants