Rewrite _preprocess for performance and customization (BREAKING)

Project.toml

@@ -1,7 +1,7 @@
 name = "PlutoPlotly"
 uuid = "8e989ff0-3d88-8e9f-f020-2b208a939ff0"
 authors = ["Alberto Mengali <[email protected]>"]
-version = "0.5.0"
+version = "0.6.0-DEV"
 
 [deps]
 AbstractPlutoDingetjes = "6e696c72-6542-2067-7265-42206c756150"

ext/UnitfulExt.jl

@@ -1,8 +1,8 @@
 module UnitfulExt
 
-using PlutoPlotly: _preprocess, PlutoPlotly
+using PlutoPlotly: _process_with_names, PlutoPlotly, AttrName
 using Unitful: Quantity, ustrip
 
-PlutoPlotly._preprocess(q::Quantity) = _preprocess(ustrip(q))
+PlutoPlotly._process_with_names(q::Quantity, fl::Val, @nospecialize(args::Vararg{AttrName})) = _process_with_names(ustrip(q), fl, args...)
 
 end

notebooks/basic_tests.jl

@@ -16,9 +16,11 @@ macro bind(def, element)
     end
 end
 
+# ╔═╡ db523d3c-df04-4722-937b-33ccf374800f
+using PlutoDevMacros
+
 # ╔═╡ 72c073fd-5f1b-4af0-901b-aaa901f0f273
 begin
-	using PlutoDevMacros
 	using PlutoUI
 	using PlutoExtras
 	using Dates
@@ -846,6 +848,7 @@ version = "17.4.0+2"
 
 # ╔═╡ Cell order:
 # ╠═7bd46437-8af0-4a15-87e9-1508869e1600
+# ╠═db523d3c-df04-4722-937b-33ccf374800f
 # ╠═72c073fd-5f1b-4af0-901b-aaa901f0f273
 # ╠═70dc8fa0-cc32-4ebe-af0d-62b5bb3a82ed
 # ╟─acba5003-a456-4c1a-a53f-71a3bec30251

src/preprocess.jl

@@ -1,96 +1,156 @@
-const SKIP_FLOAT32 = ScopedValue(false)
-skip_float32(f) = let
-    with(f, SKIP_FLOAT32 => true)
+const FORCE_FLOAT32 = ScopedValue(true)
+
+# Get a val to specify whether 
+floatval() = Val{FORCE_FLOAT32[]}()
+
+# Helper struct just to have the name as symbol parameter for dispatch
+struct AttrName{S}
+    name::Symbol
+    AttrName(s::Symbol) = new{s}(s)
 end
+# This is used to handle args... which in case only one element is passed is not iterable
+Base.iterate(n::AttrName, state = 1) = state > 1 ? nothing : (n, state + 1)
 
 #=
 This function is basically `_json_lower` from PlotlyBase, but we do it directly
 on the PlutoPlot to avoid the modifying the behavior of `_json_lower` for `Plot`
-objects (which is required to modify how matrices are passed to `publish_to_js`)
-=# 
+objects (which is required to modify how matrices are passed to `publish_to_js`).
+
+We now have a complex dispatch to be able to do custom processing for specific
+attributes.
+
+The standard signature for a _process_with_names method is:
+    _process_with_names(x, fl::Val, @nospecialize(args::Vararg{AttrName}))
+
+where 
+- the first argument should be the actual input to process and should be
+typed accordingly for dispatch.
+- The second argument is either `Val{true}` or `Val{false}` and represents the
+flag to force number to be converted in Float32. # We added this to
+significantly improve performance as the runtime check for converting or not was
+creating type instability.
+- All the remaining arguments are of type `AttrName` and represent the path of
+attributes names leading to this specific input. For example, if we are
+processing the input that is inside the xaxis_range in the layout, the function
+call will have this form:
+    _process_with_names(x, fl, AttrName(:xaxis), AttrName(:range), AttrName(:layout))
 
-# Main _preprocess for the PlutoPlot object
-function _preprocess(pp::PlutoPlot)
-	p = pp.Plot
+This again is to allow dispatch to work on the path so that one can customize behavior of _process_with_names with great control.
+At the moment this is only used for modifying the behavior when `title` is
+passed as a String, changing it to the more recent plotly syntax (see
+https://github.com/JuliaPluto/PlutoPlotly.jl/issues/51)
+
+The various `@nospecialize` below are to avoid exploding compilation given our exponential number of dispatch options, so we only specialize where we need.
+=#
+
+# Main _process_with_names for the PlutoPlot object
+function _process_with_names(pp::PlutoPlot)
+    p = pp.Plot
+    fl = floatval()
     out = Dict(
-        :data => _preprocess(p.data),
-        :layout => _preprocess(p.layout),
-        :frames => _preprocess(p.frames),
-        :config => _preprocess(p.config)
+        :data => _process_with_names(p.data, fl, AttrName(:data)),
+        :layout => _process_with_names(p.layout, fl, AttrName(:layout)),
+        :frames => _process_with_names(p.frames, fl, AttrName(:frames)),
+        :config => _process_with_names(p.config, fl, AttrName(:config))
     )
 
-	templates = PlotlyBase.templates
+    templates = PlotlyBase.templates
     layout_template = p.layout.template
     template = if layout_template isa String
         layout_template === "none" ? Template() : templates[layout_template]
     elseif layout_template === templates[templates.default]
-		# If we enter here we did not specify any template in the layout, so se use our default
+        # If we enter here we did not specify any template in the layout, so se use our default
         DEFAULT_TEMPLATE[]
     else
         layout_template
-	end
-    out[:layout][:template] = _preprocess(template)
+    end
+    out[:layout][:template] = _process_with_names(template, fl, AttrName(:template), AttrName(:layout))
     out
 end
 
-# Defaults to JSON.lower for generic non-overloaded types
-_preprocess(x) = PlotlyBase.JSON.lower(x) # Default
-_preprocess(x::TimeType) = sprint(print, x) # For handling datetimes
+# Generic fallbacks
+_process_with_names(x, ::Val, @nospecialize(args::Vararg{AttrName})) = _preprocess(x)
+_process_with_names(x) = _process_with_names(x, floatval())
 
-_preprocess(s::AbstractString) = String(s)
+# Handle strings
+_process_with_names(s::AbstractString, ::Val, @nospecialize(args::Vararg{AttrName})) =
+    _preprocess(s)
+_process_with_names(s::AbstractString, ::Val, ::AttrName{:title}, @nospecialize(args::Vararg{AttrName})) =
+    Dict(:text => _preprocess(s))
 
-_preprocess(x::Real) = SKIP_FLOAT32[] ? x : Float32(x)
+# Handle Reals
+_process_with_names(x::Real, ::Val{false}, @nospecialize(args::Vararg{AttrName})) = x
+_process_with_names(x::Real, ::Val{true}, @nospecialize(args::Vararg{AttrName})) = x isa Bool ? x : Float32(x)
+# Tuple, Arrays
+_process_with_names(x::Union{Tuple,AbstractArray}, fl::Val, @nospecialize(args::Vararg{AttrName})) = [_process_with_names(el, fl, args...) for el in x]
+# Multidimensional array of numbers must be nested 1D arrays
+_process_with_names(A::AbstractArray{<:Union{Number,AbstractVector{<:Number}},N}, fl::Val, @nospecialize(args::Vararg{AttrName})) where {N} =
+    if N == 1
+        [_process_with_names(el, fl, args...) for el in A]
+    else
+        [_process_with_names(collect(s), fl, args...) for s ∈ eachslice(A; dims=ndims(A))]
+    end
 
-_preprocess(x::Union{Bool,String,Nothing,Missing}) = x
-_preprocess(x::Symbol) = string(x)
-_preprocess(x::Union{Tuple,AbstractArray}) = _preprocess.(x)
-_preprocess(A::AbstractArray{<:Union{Number, AbstractVector{<:Number}}, N}) where N = if N == 1
-    collect(_preprocess.(A))
-else
-    [_preprocess(collect(s)) for s ∈ eachslice(A; dims = ndims(A))]
-end
-function _preprocess(d::Dict) 
-    Dict{Any,Any}(k => _preprocess(v) for (k, v) in pairs(d))
-end
-_preprocess(a::PlotlyBase.HasFields) = Dict{Any,Any}(k => _preprocess(v) for (k, v) in pairs(a.fields))
-_preprocess(c::PlotlyBase.Cycler) = c.vals
-function _preprocess(c::PlotlyBase.ColorScheme)::Vector{Tuple{Float64,String}}
-    N = length(c.colors)
-    map(ic -> ((ic[1] - 1) / (N - 1), _preprocess(ic[2])), enumerate(c.colors))
-end
+# Dict ans HasFields
+_process_with_names(d::Dict, fl::Val, @nospecialize(args::Vararg{AttrName})) =
+    Dict{Any,Any}(k => _process_with_names(v, fl, args...) for (k, v) in pairs(d))
+_process_with_names(d::Dict{Symbol}, fl::Val, @nospecialize(args::Vararg{AttrName})) =
+    Dict{Symbol,Any}(k => _process_with_names(v, fl, AttrName(k), args...) for (k, v) in pairs(d))
+# We have a separate one because it seems to reduce allocations
+_process_with_names(a::PlotlyBase.HasFields, fl::Val, @nospecialize(args::AttrName)) =
+    _process_with_names(a.fields, fl, args...)
 
-_preprocess(t::PlotlyBase.Template) = Dict(
-    :data => _preprocess(t.data),
-    :layout => _preprocess(t.layout)
+# Templates
+_process_with_names(t::PlotlyBase.Template, fl::Val, @nospecialize(args::Vararg{AttrName})) = Dict(
+    :data => _process_with_names(t.data, fl, AttrName(:data), args...),
+    :layout => _process_with_names(t.layout, fl, AttrName(:layout), args...)
 )
 
-function _preprocess(pc::PlotlyBase.PlotConfig)
+# Config
+function _process_with_names(pc::PlotlyBase.PlotConfig, fl::Val, @nospecialize(args::Vararg{AttrName}))
     out = Dict{Symbol,Any}()
     for fn in fieldnames(PlotlyBase.PlotConfig)
         field = getfield(pc, fn)
         if !isnothing(field)
-            out[fn] = field
+            out[fn] = _process_with_names(field, fl, AttrName(fn), args...)
         end
     end
     out
 end
 
+## The functions below are the internal processing only taking the value, so not depending on names path or float32 flag
+# Defaults to JSON.lower for generic non-overloaded types
+_preprocess(x) = PlotlyBase.JSON.lower(x) # Default
+_preprocess(x::TimeType) = sprint(print, x) # For handling datetimes
+
+_preprocess(s::Union{AbstractString, Symbol}) = String(s)
+
+_preprocess(x::Union{Nothing,Missing}) = x
+_preprocess(x::Symbol) = string(x)
+
+_preprocess(c::PlotlyBase.Cycler) = c.vals
+
+function _preprocess(c::PlotlyBase.ColorScheme)::Vector{Tuple{Float64,String}}
+    N = length(c.colors)
+    map(ic -> ((ic[1] - 1) / (N - 1), _preprocess(ic[2])), enumerate(c.colors))
+end
+
 # Files that will be later moved to an extension. At the moment it's pointless because PlotlyBase uses those internally anyway.
 _preprocess(s::LaTeXString) = s.s
 
 # Colors, they can be put inside an extension
 _preprocess(c::Color) = @views begin
     s = hex(c, :rrggbb)
-    r = parse(Int, s[1:2]; base = 16)
-    g = parse(Int, s[3:4]; base = 16)
-    b = parse(Int, s[5:6]; base = 16)
+    r = parse(Int, s[1:2]; base=16)
+    g = parse(Int, s[3:4]; base=16)
+    b = parse(Int, s[5:6]; base=16)
     return "rgb($r,$g,$b)"
 end
 _preprocess(c::TransparentColor) = @views begin
     s = hex(c, :rrggbbaa)
-    r = parse(Int, s[1:2]; base = 16)
-    g = parse(Int, s[3:4]; base = 16)
-    b = parse(Int, s[5:6]; base = 16)
-    a = parse(Int, s[7:8]; base = 16)
+    r = parse(Int, s[1:2]; base=16)
+    g = parse(Int, s[3:4]; base=16)
+    b = parse(Int, s[5:6]; base=16)
+    a = parse(Int, s[7:8]; base=16)
     return "rgba($r,$g,$b,$(a/255))"
 end

src/show.jl

@@ -11,7 +11,7 @@ function _show(pp::PlutoPlot; script_id = "pluto-plotly-div", ver = get_plotly_v
 		}
 
 		// Publish the plot object to JS
-		let plot_obj = _.update($(maybe_publish_to_js(_preprocess(pp))), "layout", removeTypedArray)
+		let plot_obj = _.update($(maybe_publish_to_js(_process_with_names(pp))), "layout", removeTypedArray)
 		// Get the plotly listeners
 		const plotly_listeners = $(pp.plotly_listeners)
 		// Get the JS listeners

test/basic_coverage.jl

@@ -1,14 +1,16 @@
 using Test
 using PlutoPlotly
-using PlutoPlotly: _preprocess, SKIP_FLOAT32, skip_float32, ARTIFACT_VERSION, PLOTLY_VERSION
+using PlutoPlotly: _preprocess, FORCE_FLOAT32, ARTIFACT_VERSION, PLOTLY_VERSION, _process_with_names
 using PlutoPlotly.PlotlyBase: ColorScheme, Colors, Cycler, templates
 using ScopedValues
 
-@test SKIP_FLOAT32[] == false
-@test skip_float32() do
-    SKIP_FLOAT32[]
-end == true
-@test SKIP_FLOAT32[] == false
+p = plot(rand(Int, 4));
+_p = p |> _process_with_names
+@test first(_p[:data])[:y] isa Vector{Float32}
+with(FORCE_FLOAT32 => false) do 
+    _p = p |> _process_with_names
+    @test first(_p[:data])[:y] isa Vector{Int}
+end
 
 @test force_pluto_mathjax_local() === false
 try
@@ -19,13 +21,14 @@ finally
 end
 
 @test ColorScheme([Colors.RGB(0.0, 0.0, 0.0), Colors.RGB(1.0, 1.0, 1.0)],
-"custom", "twotone, black and white") |> _preprocess == [(0.0, "rgb(0,0,0)"), (1.0, "rgb(255,255,255)")]
+"custom", "twotone, black and white") |> _process_with_names == [(0.0, "rgb(0,0,0)"), (1.0, "rgb(255,255,255)")]
 @test _preprocess(SubString("asda",1:3)) === "asd"
 @test _preprocess(:lol) === "lol"
-@test _preprocess(true) === true
+@test _process_with_names(true) === true
 @test _preprocess(Cycler((1,2))) == [1,2]
-@test _preprocess(1) === 1.0f0
+@test _process_with_names(1) === 1.0f0 # By default process converts to Float32
 @test _preprocess(L"3+2") === raw"$3+2$"
+@test all(x -> _preprocess(x) === x, [nothing, missing])
 
 # Check that plotly is the default
 @test default_plotly_template() == templates[templates.default]

test/extensions.jl

@@ -27,11 +27,11 @@ if Sys.islinux()
 end
 
 ## Unitful Extension ##
-using PlutoPlotly: _preprocess
+using PlutoPlotly: _process_with_names
 using Unitful: °, ustrip
 
 uv_r = range(0°, 100°; step = 1°)
-@test _preprocess(uv_r) == collect(0:100)
+@test _process_with_names(uv_r) == collect(0:100)
 uv_a = rand(3,5) .* °
 uv_a_strip = ustrip.(uv_a)
-@test _preprocess(uv_a) == _preprocess(uv_a_strip)
+@test _process_with_names(uv_a) == _process_with_names(uv_a_strip)