From f57f2d8079ecdf08789b0d6b1e6fa600d4506ed2 Mon Sep 17 00:00:00 2001 From: SimonDanisch Date: Wed, 11 Dec 2024 15:52:38 +0100 Subject: [PATCH 1/5] Allow for user defined recipes to be used in SpecApi --- MakieCore/src/recipes.jl | 9 +++++++++ src/specapi.jl | 8 ++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/MakieCore/src/recipes.jl b/MakieCore/src/recipes.jl index d20636519d4..8f396c12538 100644 --- a/MakieCore/src/recipes.jl +++ b/MakieCore/src/recipes.jl @@ -21,6 +21,12 @@ function plotfunc(f::Function) end end +symbol_to_plot(x::Symbol) = symbol_to_plot(Val(x)) +function symbol_to_plot(::Val{Sym}) where {Sym} + return nothing +end + + function plotfunc!(x) F = plotfunc(x)::Function name = Symbol(nameof(F), :!) @@ -188,6 +194,7 @@ macro recipe(theme_func, Tsym::Symbol, args::Symbol...) Core.@__doc__ ($funcname)(args...; kw...) = _create_plot($funcname, Dict{Symbol, Any}(kw), args...) ($funcname!)(args...; kw...) = _create_plot!($funcname, Dict{Symbol, Any}(kw), args...) $(MakieCore).default_theme(scene, ::Type{<:$PlotType}) = $(esc(theme_func))(scene) + $(MakieCore).symbol_to_plot(::Val{$(QuoteNode(Tsym))}) = $PlotType export $PlotType, $funcname, $funcname! end if !isempty(args) @@ -496,6 +503,8 @@ function create_recipe_expr(Tsym, args, attrblock) $(MakieCore).documented_attributes(::Type{<:$(PlotType)}) = $attr_placeholder $(MakieCore).plotsym(::Type{<:$(PlotType)}) = $(QuoteNode(Tsym)) + $(MakieCore).symbol_to_plot(::Val{$(QuoteNode(Tsym))}) = $PlotType + function ($funcname)(args...; kw...) kwdict = Dict{Symbol, Any}(kw) _create_plot($funcname, kwdict, args...) diff --git a/src/specapi.jl b/src/specapi.jl index 45e81923489..77f7199da4d 100644 --- a/src/specapi.jl +++ b/src/specapi.jl @@ -3,6 +3,7 @@ using GridLayoutBase: GridLayoutBase import GridLayoutBase: GridPosition, Side, ContentSize, GapSize, AlignMode, Inner, GridLayout, GridSubposition + function get_recipe_function(name::Symbol) if hasproperty(Makie, name) return getfield(Makie, name) @@ -169,7 +170,7 @@ function to_plotspec(::Type{P}, p::PlotSpec; kwargs...) where {P} return PlotSpec(plotsym(plottype(P, S)), p.args...; p.kwargs..., kwargs...) end -plottype(p::PlotSpec) = getfield(Makie, p.type) +plottype(p::PlotSpec) = MakieCore.symbol_to_plot(p.type) function Base.show(io::IO, ::MIME"text/plain", spec::PlotSpec) args = join(map(x -> string("::", typeof(x)), spec.args), ", ") @@ -403,7 +404,10 @@ function Base.getproperty(::_SpecApi, field::Symbol) # Since precompilation will cache only MakieCore's state # And once everything is compiled, and MakieCore is loaded into a package # The names are loaded from cache and dont contain anything after MakieCore. - func = get_recipe_function(field) + func = MakieCore.symbol_to_plot(field) + if isnothing(func) + func = get_recipe_function(field) + end if isnothing(func) error("$(field) neither a recipe, Makie plotting object or a Block (like Axis, Legend, etc).") elseif func isa Function From e99aa1a4c0e5ef07ff58fadd8b3d422062f22c2f Mon Sep 17 00:00:00 2001 From: Simon Date: Wed, 11 Dec 2024 17:30:42 +0100 Subject: [PATCH 2/5] Update CHANGELOG.md --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f883599a4ac..57e6db7cfad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## [Unreleased] +- Allow for user defined recipes to be used in SpecApi [#4655](https://github.com/MakieOrg/Makie.jl/pull/4655). + ## [0.21.17] - 2024-12-05 - Added `backend` and `update` kwargs to `show` [#4558](https://github.com/MakieOrg/Makie.jl/pull/4558) From 33c08aff8c11b76c4ee560303a8308131e90d2b9 Mon Sep 17 00:00:00 2001 From: Simon Date: Wed, 11 Dec 2024 19:11:43 +0100 Subject: [PATCH 3/5] allow external blocks as well --- src/makielayout/blocks.jl | 17 ++++++++++------- src/specapi.jl | 20 +++++++------------- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/src/makielayout/blocks.jl b/src/makielayout/blocks.jl index d90f2921731..b37780777e8 100644 --- a/src/makielayout/blocks.jl +++ b/src/makielayout/blocks.jl @@ -6,6 +6,9 @@ function attribute_default_expressions end function _attribute_docs end function has_forwarded_layout end +symbol_to_block(symbol::Symbol) = symbol_to_block(Val(symbol)) +symbol_to_block(::Val) = nothing + macro Block(_name::Union{Expr, Symbol}, body::Expr = Expr(:block)) body.head === :block || error("A Block needs to be defined within a `begin end` block") @@ -78,18 +81,18 @@ macro Block(_name::Union{Expr, Symbol}, body::Expr = Expr(:block)) $structdef export $name - - function Makie.is_attribute(::Type{$(name)}, sym::Symbol) + $(Makie).symbol_to_block(::Val{$(QuoteNode(name))}) = $name + function $(Makie).is_attribute(::Type{$(name)}, sym::Symbol) sym in ($((attrs !== nothing ? [QuoteNode(a.symbol) for a in attrs] : [])...),) end - function Makie.default_attribute_values(::Type{$(name)}, scene::Union{Scene, Nothing}) + function $(Makie).default_attribute_values(::Type{$(name)}, scene::Union{Scene, Nothing}) sceneattrs = scene === nothing ? Attributes() : theme(scene) - curdeftheme = Makie.fast_deepcopy($(Makie).CURRENT_DEFAULT_THEME) + curdeftheme = $(Makie).fast_deepcopy($(Makie).CURRENT_DEFAULT_THEME) $(make_attr_dict_expr(attrs, :sceneattrs, :curdeftheme)) end - function Makie.attribute_default_expressions(::Type{$name}) + function $(Makie).attribute_default_expressions(::Type{$name}) $( if attrs === nothing Dict{Symbol, String}() @@ -99,7 +102,7 @@ macro Block(_name::Union{Expr, Symbol}, body::Expr = Expr(:block)) ) end - function Makie._attribute_docs(::Type{$(name)}) + function $(Makie)._attribute_docs(::Type{$(name)}) Dict( $( (attrs !== nothing ? @@ -109,7 +112,7 @@ macro Block(_name::Union{Expr, Symbol}, body::Expr = Expr(:block)) ) end - Makie.has_forwarded_layout(::Type{$name}) = $has_forwarded_layout + $(Makie).has_forwarded_layout(::Type{$name}) = $has_forwarded_layout docstring_modified = make_block_docstring($name, user_docstring) @doc docstring_modified $name diff --git a/src/specapi.jl b/src/specapi.jl index 77f7199da4d..1a334abad17 100644 --- a/src/specapi.jl +++ b/src/specapi.jl @@ -3,13 +3,10 @@ using GridLayoutBase: GridLayoutBase import GridLayoutBase: GridPosition, Side, ContentSize, GapSize, AlignMode, Inner, GridLayout, GridSubposition - -function get_recipe_function(name::Symbol) - if hasproperty(Makie, name) - return getfield(Makie, name) - else - return nothing - end +function symbol_to_specable(sym::Symbol) + block = symbol_to_block(sym) + isnothing(block) || return block + return MakieCore.symbol_to_plot(sym) end """ @@ -28,7 +25,7 @@ struct PlotSpec error("PlotSpec objects are supposed to be used without !, unless when using `S.$(type)(axis::P.Axis, args...; kwargs...)`") end if !isuppercase(type_str[1]) - func = get_recipe_function(type) + func = hasproperty(Makie, type) ? getproperty(Makie, type) : nothing func === nothing && error("PlotSpec need to be existing recipes or Makie plot objects. Found: $(type_str)") plot_type = Plot{func} type = plotsym(plot_type) @@ -404,10 +401,7 @@ function Base.getproperty(::_SpecApi, field::Symbol) # Since precompilation will cache only MakieCore's state # And once everything is compiled, and MakieCore is loaded into a package # The names are loaded from cache and dont contain anything after MakieCore. - func = MakieCore.symbol_to_plot(field) - if isnothing(func) - func = get_recipe_function(field) - end + func = symbol_to_specable(field) if isnothing(func) error("$(field) neither a recipe, Makie plotting object or a Block (like Axis, Legend, etc).") elseif func isa Function @@ -748,7 +742,7 @@ function extract_colorbar_kw(legend::BlockSpec, scene::Scene) end function to_layoutable(parent, position::GridLayoutPosition, spec::BlockSpec) - BType = getfield(Makie, spec.type) + BType = symbol_to_block(spec.type) fig = get_top_parent(parent) block = if spec.type === :Colorbar From 4e65c8a9c1c0b5799c3cfe6410866b36081f2e52 Mon Sep 17 00:00:00 2001 From: Anshul Singhvi Date: Thu, 12 Dec 2024 07:23:07 +0800 Subject: [PATCH 4/5] add basic tests for specapi with external recipes --- test/specapi.jl | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/specapi.jl b/test/specapi.jl index 9c0a6f23ac4..33aa16f3f9a 100644 --- a/test/specapi.jl +++ b/test/specapi.jl @@ -187,3 +187,13 @@ end # @test contents(contents(leg.grid)[1])[2].text[] == "A" # @test contents(contents(leg.grid)[2])[4].text[] == "B" end + + +@testset "External Recipe compatibility (#4295)" + @recipe(TestRecipeForSpecApi) do scene + return Attributes() + end + + @test_nowarn S.TestRecipeForSpecApi + @test_nowarn S.TestRecipeForSpecApi(1, 2, 3; a = 4, b = 5) +end From 87e7b5ee12685790e4cab1888fcdb356ecb3a4ba Mon Sep 17 00:00:00 2001 From: SimonDanisch Date: Thu, 12 Dec 2024 10:38:25 +0100 Subject: [PATCH 5/5] fix test --- test/specapi.jl | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/test/specapi.jl b/test/specapi.jl index 33aa16f3f9a..7f1393756fb 100644 --- a/test/specapi.jl +++ b/test/specapi.jl @@ -181,19 +181,18 @@ end # This is too internal and fragile, so we won't actually test this # @test leg.scene.plots[2].marker[] == :circle # @test leg.scene.plots[3].marker[] == :rect - + # Test that the legend has the correct labels. # Again, I consider this too fragile to work with! # @test contents(contents(leg.grid)[1])[2].text[] == "A" # @test contents(contents(leg.grid)[2])[4].text[] == "B" end - -@testset "External Recipe compatibility (#4295)" - @recipe(TestRecipeForSpecApi) do scene +@recipe(TestRecipeForSpecApi) do scene return Attributes() - end +end +@testset "External Recipe compatibility (#4295)" begin @test_nowarn S.TestRecipeForSpecApi @test_nowarn S.TestRecipeForSpecApi(1, 2, 3; a = 4, b = 5) end