PumasAI / QuartoNotebookRunner.jl

MIT License
63 stars 10 forks source link

Pdf plotting is broken (worked on iJulia) #128

Closed lrnv closed 5 months ago

lrnv commented 6 months ago

Conside the follwoing:

---
title: "title"
engine: julia
format: beamer # or pdf
---

# Intro

## Slide working 
bla bla 

## Slide not working 

```{julia}
#| echo: false
#| fig-cap: "my caption"
using Random, Plots
x = rand(2,1000)
scatter(x[1,:],x[2,:])

Produces: 

Error showing value of type Plots.Plot{Plots.GRBackend} only png or svg allowed. got: :pdf Stacktrace: [1] error(s::String) @ Base .\error.jl:35 [2] _show(io::IOContext{IOBuffer}, ::MIME{Symbol("text/html")}, plt::Plots.Plot{Plots.GRBackend}) @ Plots [...]\src\output.jl:203 [3] #invokelatest#2 @ .\essentials.jl:887 [inlined] [4] invokelatest @ .\essentials.jl:884 [inlined] [5] show(io::IOContext{IOBuffer}, m::MIME{Symbol("text/html")}, plt::Plots.Plot{Plots.GRBackend}) @ Plots [...]\src\output.jl:232 [6] show(io::IOContext{IOBuffer}, m::String, x::Plots.Plot{Plots.GRBackend}) @ Base.Multimedia .\multimedia.jl:123



Is there a quick workaround ?
lrnv commented 6 months ago

Looks related to https://github.com/quarto-dev/quarto-cli/issues/722

jkrumbiegel commented 6 months ago

Why is this coming from a show method that renders text/html though I wonder. There is no :pdf symbol in our code base so at least we're not setting that.

lrnv commented 6 months ago

Indeed, very good question. I guess htere has been some kind of issues with that already on the jupyter-julia version, I think that somewhere quarto is asking for the plot to be rendered with weird inputs...

MichaelHatherly commented 6 months ago

Perhaps happening here:

https://github.com/PumasAI/QuartoNotebookRunner.jl/blob/554b66034d0ece0a91bcd9dbc2e1e9dd36d2018b/src/worker.jl#L785-L789

MHellmund commented 6 months ago

It seems that the support for pdf output in Plots.jl >= 1.28.1 is intended to be for IJulia only:

https://github.com/JuliaPlots/Plots.jl/compare/v1.28.0...v1.28.1 , bottom of page

Outside of IJulia, allowed values for fmt are png and svg: https://docs.juliaplots.org/stable/generated/attributes_plot/

With the new QuartoNotebookWorker, what is the canonical way for a Quarto user to modify the package integrations in src/QuartoNotebookWorker/ext?

MichaelHatherly commented 6 months ago

what is the canonical way for a Quarto user to modify the package integrations

It'll depend on which and what is needing modification. For a quarto user, it's via the frontmatter, and that will modify the behaviours, so no change compared to previous non-QuartoNotebookWorker.

A temp "solution" so that pdf at least doesn't error would be to use :png. Though I don't particularly like that since the quality suffers.

jkrumbiegel commented 6 months ago

Doesn't it actually look like our code was already supposed to catch this bug by choosing png if "pdf" was specified? We copied that from somewhere else I think, where this workaround was already in use, maybe the quarto julia jupyter engine. But the error specifies that :pdf is not known, which is different from "pdf", thereby circumventing that path. Maybe we just need to check the value correctly for now.

MichaelHatherly commented 6 months ago

Directly from https://github.com/quarto-dev/quarto-cli/blob/e1026cf2cc1c91febbf4631b38f91ef7848f4f33/src/resources/jupyter/lang/julia/setup.jl

jkrumbiegel commented 6 months ago

Right, look here, they interpolated that string variable with a leading : making it a Symbol, that's the difference

https://github.com/quarto-dev/quarto-cli/blob/e1026cf2cc1c91febbf4631b38f91ef7848f4f33/src/resources/jupyter/lang/julia/setup.jl#L16

lrnv commented 5 months ago

Thans a lot @MichaelHatherly I will come back if i have any other issues :)