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

Fix PDF render #1038

Closed
wants to merge 1 commit into from
Closed

Conversation

jeremiahpslewis
Copy link

@jeremiahpslewis jeremiahpslewis commented Apr 29, 2022

Fixes #1035 alongside

PR #4188 (which has been merged)

@jeremiahpslewis
Copy link
Author

@fredrikekre Wanted to flag this as ready for review. :)

@stevengj
Copy link
Member

stevengj commented May 3, 2022

Is display of PDF images well supported in Jupyter?

This issue jupyter/notebook#3652 seems to indicate that it only works in certain browsers or with certain plug-ins installed.

@jeremiahpslewis
Copy link
Author

jeremiahpslewis commented May 4, 2022

I think you're right in pointing out there is a bit of a file type conflict here:

Per the issue you linked, browser support of PDFs is (potentially) incomplete.

But Latex workflows like quarto's use case which use Jupyter to build a document (but not display it in the browser) do not work well with SVGs and require the use of the PDF format for exporting / displaying vector graphics.

Right now, if you create a PDF document with R, Python, and Julia graphs via quarto, the Julia graph gets dumped out as a PNG and looks significantly pixelated, the other two languages (afaik) allow for PDF viz export and certainly lead to vector viz output in the latex PDF document.

tldr ruling out support of PDFs because it does not work everywhere, perfectly, cuts out meaningful use cases for (quarto) Latex documents; R and Python allow this

@twavv
Copy link
Contributor

twavv commented May 4, 2022

It's worth noting that you can display the PDF "manually" using display("application/pdf", x). Looking at JupyterLab, looks like you need the @jupyterlab/pdf-extension installed.

I don't think it should be added by default. It has higher priority than image/xyz, which is probably unexpected for most use cases, and will also increase the notebook file sizes (Jupyter stores all different supported formats of a file in the notebook, not just one).

If Quarto really want's to customize this, they can define a custom method for IJulia.display_dict that includes the PDF.

@jeremiahpslewis
Copy link
Author

jeremiahpslewis commented May 4, 2022

@travigd Thanks for the feedback and flagging potential downsides to this change. Can you say more about the cases in which Jupyter would actually generate multiple file formats for a given cell as a result of adding specifically application/pdf?

My understanding (please correct):

For the Plots.jl usecase, at any point in time there should only be one active backend, with one format for that backend. So only in the case that your active backend, say gr is targeting pdf's are you going to get a PDF output in that cell. Same story for Makie.jl.

I can't come up with a case in which you'd see multiple outputs, and certainly in the case of the PDF plots which this PR seeks to open up as a possibility, to my knowledge you're really talking about a sort of wrapped svg, so not very large file sizes for typical cases.

While it certainly would be possible to overload a function or patch IJulia, this creates other (minor?) types of unpleasantness should IJulia change the code base.

@twavv
Copy link
Contributor

twavv commented May 4, 2022

The Jupyter format defines cell outputs as a dict that contains multiple representations of an output (e.g., plot). The frontend (e.g., jupyterlab) chooses which of the formats its want to render (so only one is shown at a time, but the various formats are present in the notebook file).

IJulia tries to generate a representation for every MIME type it supports (which is what this PR is changing).

This is actually not the case for Plots.jl -- it actually does override IJulia.display_dict: https://github.com/JuliaPlots/Plots.jl/blob/b6d7bd82e6c029b8cbbafa756015670cb0ab0013/src/init.jl#L92, which is what (personally) I think Quarto should do.

@jeremiahpslewis
Copy link
Author

@travigd Thanks for the nudge to investigate this further, the Plots.jl PR JuliaPlots/Plots.jl#4188 solved the issue for Plots, so this isn't necessary.

And for Makie, turns out it's a one-liner which can be done on the Quarto side: IJulia.register_mime(MIME("application/pdf")) 🎉

@benz0li
Copy link

benz0li commented Jul 25, 2022

This is actually not the case for Plots.jl

Is there a way to override this in startup.jl/startup_ijulia.jl? For

  1. Python, one may set c.InlineBackend.figure_formats = {'svg', 'pdf'} in ipython_config.py/ipython_kernel_config.py
  2. R, one may set options(jupyter.plot_mimetypes = c('text/plain', 'image/svg+xml', 'application/pdf')) in Rprofile.site

in order to output both image/svg+xml and application/pdf plot formats in the notebook file:

  • image/svg+xml for plot display in the browser
    + HTML conversion using SVG instead of PNG
  • application/pdf for direct PDF conversion

ℹ️ With application/pdf plot format in the notebook file, inkscape is not required for PDF conversion using nbconvert.

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.

support for pdf plot output
4 participants