From 7ab25837d143140d12465215123ce2a872d1f136 Mon Sep 17 00:00:00 2001 From: TEC Date: Sun, 26 May 2024 12:00:21 +0800 Subject: [PATCH] refactor(advice): remove adviseall field Also no longer implement a special getproperty method. Instead we can just call a dedicated function when appropriate. --- src/model/advice.jl | 60 +++++++++++++++++++++++++++------------------ src/model/types.jl | 4 +-- test/runtests.jl | 21 ++++++---------- 3 files changed, 45 insertions(+), 40 deletions(-) diff --git a/src/model/advice.jl b/src/model/advice.jl index b2a358b7..89bead48 100644 --- a/src/model/advice.jl +++ b/src/model/advice.jl @@ -8,8 +8,8 @@ Base.methods(dt::Advice) = methods(dt.f) Apply `advice` to the function call `func(args...; kwargs...)`, and return the new `(post, func, args, kwargs)` tuple. """ -function (dt::Advice{F, C})(callform::Tuple{Function, Function, Tuple, NamedTuple}) where {F, C} - @nospecialize callform +function (dt::Advice)(callform::Tuple{Function, Function, Tuple, NamedTuple}) + @nospecialize post, func, args, kwargs = callform # Abstract-y `typeof`. atypeof(val::Any) = typeof(val) @@ -54,30 +54,37 @@ end Base.empty(::Type{AdviceAmalgamation}) = - AdviceAmalgamation(identity, Advice[], String[], String[]) - -# When getting a property of a `AdviceAmalgamation`, first check -# if the `:plugins_wanted` field is satisfied. Should it not be, -# regenerate the `:advisors`, `:adviseall`, and `:plugins_used` fields -# based on the currently available plugins and `:plugins_wanted`. -function Base.getproperty(dta::AdviceAmalgamation, prop::Symbol) - if getfield(dta, :plugins_wanted) != getfield(dta, :plugins_used) + AdviceAmalgamation(Advice[], String[], String[]) + +""" + reinit(dta::AdviceAmalgamation) + +Check that `dta` is well initialised before using it. + +This does noting if `dta.plugins_wanted` is the same as `dta.plugins_used`. + +When they differ, it re-builds the advisors function list based +on the currently available plugins, and updates `dta.plugins_used`. +""" +function reinit(dta::AdviceAmalgamation) + if dta.plugins_wanted != dta.plugins_used plugins_available = - filter(plugin -> plugin.name in getfield(dta, :plugins_wanted), PLUGINS) - if getfield.(plugins_available, :name) != getfield(dta, :plugins_used) - advisors = getfield.(plugins_available, :advisors) |> - Iterators.flatten |> collect |> Vector{Advice} + filter(plugin -> plugin.name in dta.plugins_wanted, PLUGINS) + if map(p -> p.name, plugins_available) != dta.plugins_used + advisors = Advice[] + for plg in plugins_available + append!(advisors, plg.advisors) + end sort!(advisors, by = t -> t.priority) - setfield!(dta, :advisors, advisors) - setfield!(dta, :adviseall, ∘(reverse(advisors)...)) - setfield!(dta, :plugins_used, getfield.(plugins_available, :name)) + dta.advisors = advisors + dta.plugins_used = map(p -> p.name, plugins_available) end end - getfield(dta, prop) + dta end AdviceAmalgamation(plugins::Vector{String}) = - AdviceAmalgamation(identity, Advice[], plugins, String[]) + AdviceAmalgamation(Advice[], plugins, String[]) AdviceAmalgamation(collection::DataCollection) = AdviceAmalgamation(collection.plugins) @@ -87,16 +94,21 @@ AdviceAmalgamation(dta::AdviceAmalgamation) = # for re-building function AdviceAmalgamation(advisors::Vector{<:Advice}) advisors = sort(advisors, by = t -> t.priority) - AdviceAmalgamation(∘(reverse(advisors)...), advisors, String[], String[]) + AdviceAmalgamation(advisors, String[], String[]) end -function (dta::AdviceAmalgamation)(@nospecialize( - annotated_func_call::Tuple{Function, Function, Tuple, NamedTuple})) - dta.adviseall(annotated_func_call) +function (dta::AdviceAmalgamation)(annotated_func_call::Tuple{Function, Function, Tuple, NamedTuple}) + @nospecialize + reinit(dta) + for adv in dta.advisors + annotated_func_call = adv(annotated_func_call) + end + annotated_func_call end function (dta::AdviceAmalgamation)(func::Function, args...; kwargs...) - @nospecialize func args kwargs + @nospecialize + reinit(dta) post::Function, func2::Function, args2::Tuple, kwargs2::NamedTuple = dta((identity, func, args, merge(NamedTuple(), kwargs))) invokepkglatest(func2, args2...; kwargs2...) |> post diff --git a/src/model/types.jl b/src/model/types.jl index c8b0a337..6359493e 100644 --- a/src/model/types.jl +++ b/src/model/types.jl @@ -264,14 +264,12 @@ as a function. However, it also supports the following convenience syntax: # Constructors ``` -AdviceAmalgamation(adviseall::Function, advisors::Vector{Advice}, - plugins_wanted::Vector{String}, plugins_used::Vector{String}) +AdviceAmalgamation(advisors::Vector{Advice}, plugins_wanted::Vector{String}, plugins_used::Vector{String}) AdviceAmalgamation(plugins::Vector{String}) AdviceAmalgamation(collection::DataCollection) ``` """ mutable struct AdviceAmalgamation - adviseall::Function advisors::Vector{Advice} plugins_wanted::Vector{String} plugins_used::Vector{String} diff --git a/test/runtests.jl b/test/runtests.jl index 228b42b3..48c12f3a 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -4,7 +4,7 @@ using Test import DataToolkitBase: natkeygen, stringdist, stringsimilarity, longest_common_subsequence, highlight_lcs, referenced_datasets, stack_index, plugin_add, plugin_list, plugin_remove, config_get, - config_set, config_unset, DATASET_REFERENCE_WRAPPER + config_set, config_unset, reinit, DATASET_REFERENCE_WRAPPER @testset "Utils" begin @testset "Doctests" begin @@ -88,16 +88,12 @@ end end end @testset "Amalgamation" begin - amlg12 = AdviceAmalgamation( - sump1 ∘ sumx2, [sumx2, sump1], String[], String[]) + amlg12 = AdviceAmalgamation([sumx2, sump1], String[], String[]) @test amlg12.advisors == AdviceAmalgamation([sumx2, sump1]).advisors @test AdviceAmalgamation(amlg12).advisors == Advice[] # no plugins - amlg21 = AdviceAmalgamation( - sumx2 ∘ sump1, [sump1, sumx2], String[], String[]) - amlg321 = AdviceAmalgamation( - summ3 ∘ sumx2 ∘ sump1, [sump1, sumx2, summ3], String[], String[]) - amlg213 = AdviceAmalgamation( - sumx2 ∘ sump1 ∘ summ3, [summ3, sump1, sumx2], String[], String[]) + amlg21 = AdviceAmalgamation([sump1, sumx2], String[], String[]) + amlg321 = AdviceAmalgamation([sump1, sumx2, summ3], String[], String[]) + amlg213 = AdviceAmalgamation([summ3, sump1, sumx2], String[], String[]) @test amlg12((identity, sum, (2,), (;))) == (identity, sum, (5,), (;)) @test amlg12(sum, 2) == 5 @test amlg21(sum, 2) == 6 @@ -107,7 +103,6 @@ end @testset "Plugin loading" begin # Empty state amlg = empty(AdviceAmalgamation) - @test amlg.adviseall == identity @test amlg.advisors == Advice[] @test amlg.plugins_wanted == String[] @test amlg.plugins_used == String[] @@ -117,14 +112,14 @@ end @test Plugin("", [sumx2.f]).advisors == Plugin("", [sumx2]).advisors # Desire the plugin, then check the advice is incorperated correctly push!(amlg.plugins_wanted, plg.name) - @test amlg.adviseall == sump1 ∘ sumx2 + @test amlg(identity, 1) == 1 # Should call `reinit` @test amlg.advisors == [sumx2, sump1] @test amlg.plugins_wanted == [plg.name] @test amlg.plugins_used == [plg.name] - @test AdviceAmalgamation(amlg).advisors == amlg.advisors + @test reinit(AdviceAmalgamation(amlg)).advisors == amlg.advisors let cltn = DataCollection() push!(cltn.plugins, plg.name) - @test AdviceAmalgamation(cltn).advisors == amlg.advisors + @test reinit(AdviceAmalgamation(cltn)).advisors == amlg.advisors end # Display @test sprint(show, amlg) == "AdviceAmalgamation($(plg.name) ✔)"