From a9cd5e698857c813c150dcafb1cd69be09fdbd0b Mon Sep 17 00:00:00 2001 From: MichaelHatherly Date: Tue, 5 Mar 2024 12:56:58 +0000 Subject: [PATCH 1/6] Fix #51 Render inline HTML for plotly plots rather than complete HTML documents. --- src/worker.jl | 89 +++++++++++++++++++ test/examples/integrations/PlotlyJS.qmd | 21 +++++ .../integrations/PlotlyJS/Project.toml | 2 + test/testsets/integrations/PlotlyJS.jl | 15 ++++ 4 files changed, 127 insertions(+) create mode 100644 test/examples/integrations/PlotlyJS.qmd create mode 100644 test/examples/integrations/PlotlyJS/Project.toml create mode 100644 test/testsets/integrations/PlotlyJS.jl diff --git a/src/worker.jl b/src/worker.jl index a1de467..1dbf3d9 100644 --- a/src/worker.jl +++ b/src/worker.jl @@ -280,7 +280,42 @@ function worker_init(f::File) return take!(buf) end + # TODO: bit of a hack, a better way of doing this all would be with a + # local package that defines package extensions which we load into the + # worker processes by adjusting the `LOAD_PATH`. + const MIMETYPE_WRAPPERS = IdDict() + function _mimetype_wrapper(value::T) where {T} + key = getfield(parentmodule(T), nameof(T)) + return get(MIMETYPE_WRAPPERS, key, identity)(value) + end + function _register_wrapper(from, to) + if haskey(MIMETYPE_WRAPPERS, from) + previous_to = MIMETYPE_WRAPPERS[from] + if previous_to !== to + error( + "attempted to overwrite existing wrapper for $from: $previous_to -> $to", + ) + end + else + MIMETYPE_WRAPPERS[from] = to + end + return nothing + end + + abstract type WrapperType end + + # Required methods to avoid `show` method ambiguity errors. + Base.show(io::IO, w::WrapperType) = Base.show(io, w.value) + Base.show(io::IO, m::MIME, w::WrapperType) = Base.show(io, m, w.value) + Base.show(io::IO, m::MIME"text/plain", w::WrapperType) = Base.show(io, m, w.value) + Base.showable(mime::MIME, w::WrapperType) = Base.showable(mime, w.value) + function render_mimetypes(value, cell_options) + # Intercept objects prior to rendering so that we can wrap specific + # types in our own `WrapperType` to customised rendering instead of + # what the package defines itself. + value = _mimetype_wrapper(value) + to_format = OPTIONS[]["format"]["pandoc"]["to"] result = Dict{String,@NamedTuple{error::Bool, data::Vector{UInt8}}}() @@ -534,6 +569,50 @@ function worker_init(f::File) end _Plots_hook(::Any...) = nothing + # PlotlyBase.jl integrations: + + function _PlotlyBase_hook(pkgid::Base.PkgId, PlotlyBase::Module) + _register_wrapper(PlotlyBase.Plot, PlotlyBasePlot) + return nothing + end + _PlotlyBase_hook(::Any...) = nothing + + struct PlotlyBasePlot <: WrapperType + value::Any + end + + function Base.show(io::IO, mime::MIME"text/html", wrapper::PlotlyBasePlot) + T = typeof(wrapper.value) + PlotlyBase = parentmodule(T) + # We want to embed only the minimum markup needed to render the + # plotlyjs plots, otherwise a full HTML page is generated for every + # plot which does not render correctly in our context. + PlotlyBase.to_html( + io, + wrapper.value; + include_plotlyjs = "require", + full_html = false, + ) + end + + # PlotlyJS.jl integrations: + + function _PlotlyJS_hook(pkgid::Base.PkgId, PlotlyJS::Module) + _register_wrapper(PlotlyJS.SyncPlot, PlotlyJSSyncPlot) + return nothing + end + _PlotlyJS_hook(::Any...) = nothing + + struct PlotlyJSSyncPlot <: WrapperType + value::Any + end + + function Base.show(io::IO, mime::MIME"text/html", wrapper::PlotlyJSSyncPlot) + return Base.show(io, mime, PlotlyBasePlot(wrapper.value.plot)) + end + + # Loading hooks: + const PACKAGE_LOADING_HOOKS = Dict{Base.PkgId,Function}() if isdefined(Base, :package_callbacks) let @@ -564,6 +643,16 @@ function worker_init(f::File) "Plots", "91a5bcdd-55d7-5caf-9e0b-520d859cae80", ), + package_loading_hook!( + _PlotlyBase_hook, + "PlotlyBase", + "a03496cd-edff-5a9b-9e67-9cda94a718b5", + ) + package_loading_hook!( + _PlotlyJS_hook, + "PlotlyJS", + "f0f68f2c-4968-5e81-91da-67840de0976a", + ) push!( Base.package_callbacks, function (pkgid) diff --git a/test/examples/integrations/PlotlyJS.qmd b/test/examples/integrations/PlotlyJS.qmd new file mode 100644 index 0000000..4743acc --- /dev/null +++ b/test/examples/integrations/PlotlyJS.qmd @@ -0,0 +1,21 @@ +--- +title: PlotlyJS +julia: + exeflags: ["--project=PlotlyJS"] +--- + +```{julia} +using PlotlyJS +``` + +```{julia} +#| label: fig-scatter +#| fig-cap: "Scatter Plot" +PlotlyJS.Plot(scatter(; y = [1, 2, 3], mode = "markers")) +``` + +```{julia} +#| label: fig-line-plot +#| fig-cap: "Line Plot" +plot(scatter(; y=[1, 2, 3], mode="lines")) +``` diff --git a/test/examples/integrations/PlotlyJS/Project.toml b/test/examples/integrations/PlotlyJS/Project.toml new file mode 100644 index 0000000..681410d --- /dev/null +++ b/test/examples/integrations/PlotlyJS/Project.toml @@ -0,0 +1,2 @@ +[deps] +PlotlyJS = "f0f68f2c-4968-5e81-91da-67840de0976a" diff --git a/test/testsets/integrations/PlotlyJS.jl b/test/testsets/integrations/PlotlyJS.jl new file mode 100644 index 0000000..0587287 --- /dev/null +++ b/test/testsets/integrations/PlotlyJS.jl @@ -0,0 +1,15 @@ +include("../../utilities/prelude.jl") + +test_example(joinpath(@__DIR__, "../../examples/integrations/PlotlyJS.qmd")) do json + cells = json["cells"] + for nth in (4, 6) + cell = cells[nth] + outputs = cell["outputs"] + @test length(outputs) == 1 + data = outputs[1]["data"] + @test haskey(data, "image/png") + @test haskey(data, "image/svg+xml") + @test haskey(data, "text/html") + @test startswith(data["text/html"], "
") + end +end From 63a522ea1e16d12c20a732b741614a36ba668e30 Mon Sep 17 00:00:00 2001 From: MichaelHatherly Date: Tue, 5 Mar 2024 15:12:22 +0000 Subject: [PATCH 2/6] Disable json mime type rendering --- src/worker.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/worker.jl b/src/worker.jl index 1dbf3d9..fba31c2 100644 --- a/src/worker.jl +++ b/src/worker.jl @@ -357,7 +357,7 @@ function worker_init(f::File) "text/latex", "image/svg+xml", "image/png", - "application/json", + # "application/json", ] end for mime in mimes From 94b73be7d7b95dbc7c2d78d66892603556632fde Mon Sep 17 00:00:00 2001 From: MichaelHatherly Date: Tue, 5 Mar 2024 16:35:36 +0000 Subject: [PATCH 3/6] Try doing a GC after each testset --- src/worker.jl | 2 +- test/utilities/prelude.jl | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/worker.jl b/src/worker.jl index fba31c2..1dbf3d9 100644 --- a/src/worker.jl +++ b/src/worker.jl @@ -357,7 +357,7 @@ function worker_init(f::File) "text/latex", "image/svg+xml", "image/png", - # "application/json", + "application/json", ] end for mime in mimes diff --git a/test/utilities/prelude.jl b/test/utilities/prelude.jl index 1cbf1dc..4494906 100644 --- a/test/utilities/prelude.jl +++ b/test/utilities/prelude.jl @@ -65,5 +65,6 @@ if !@isdefined(SCHEMA) end QuartoNotebookRunner.close!(server, each) end + GC.gc() end end From 836241637583402cf6197b686dfce506a29e81c1 Mon Sep 17 00:00:00 2001 From: MichaelHatherly Date: Tue, 5 Mar 2024 17:16:42 +0000 Subject: [PATCH 4/6] Skip plotly tests on Windows Potential upstream issue. --- test/testsets/integrations/PlotlyJS.jl | 28 ++++++++++++++++---------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/test/testsets/integrations/PlotlyJS.jl b/test/testsets/integrations/PlotlyJS.jl index 0587287..809763b 100644 --- a/test/testsets/integrations/PlotlyJS.jl +++ b/test/testsets/integrations/PlotlyJS.jl @@ -1,15 +1,21 @@ include("../../utilities/prelude.jl") -test_example(joinpath(@__DIR__, "../../examples/integrations/PlotlyJS.qmd")) do json - cells = json["cells"] - for nth in (4, 6) - cell = cells[nth] - outputs = cell["outputs"] - @test length(outputs) == 1 - data = outputs[1]["data"] - @test haskey(data, "image/png") - @test haskey(data, "image/svg+xml") - @test haskey(data, "text/html") - @test startswith(data["text/html"], "
") +if Sys.iswindows() + # There are issues with the Kaleido_jll v0.2 build on Windows, skip for now. + @info "Skipping PlotlyJS.jl tests on Windows" +else + test_example(joinpath(@__DIR__, "../../examples/integrations/PlotlyJS.qmd")) do json + cells = json["cells"] + for nth in (4, 6) + cell = cells[nth] + outputs = cell["outputs"] + JSON3.pretty(outputs) + @test length(outputs) == 1 + data = outputs[1]["data"] + @test haskey(data, "image/png") + @test haskey(data, "image/svg+xml") + @test haskey(data, "text/html") + @test startswith(data["text/html"], "
") + end end end From f610cc590d1e268f5913549a0e32d86494cb229d Mon Sep 17 00:00:00 2001 From: MichaelHatherly Date: Wed, 6 Mar 2024 08:34:43 +0000 Subject: [PATCH 5/6] Remove debug printing --- test/testsets/integrations/PlotlyJS.jl | 1 - 1 file changed, 1 deletion(-) diff --git a/test/testsets/integrations/PlotlyJS.jl b/test/testsets/integrations/PlotlyJS.jl index 809763b..e98c132 100644 --- a/test/testsets/integrations/PlotlyJS.jl +++ b/test/testsets/integrations/PlotlyJS.jl @@ -9,7 +9,6 @@ else for nth in (4, 6) cell = cells[nth] outputs = cell["outputs"] - JSON3.pretty(outputs) @test length(outputs) == 1 data = outputs[1]["data"] @test haskey(data, "image/png") From 0f612ad4b0da259698dc4c067dcd6cd84dc78272 Mon Sep 17 00:00:00 2001 From: MichaelHatherly Date: Wed, 6 Mar 2024 09:36:52 +0000 Subject: [PATCH 6/6] Increase CI timeout --- .github/workflows/CI.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index fc04afd..ca33bc9 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -38,7 +38,7 @@ jobs: actions: write contents: read runs-on: ${{ matrix.os }} - timeout-minutes: 60 + timeout-minutes: 90 strategy: fail-fast: false matrix: