Skip to content
This repository has been archived by the owner on May 29, 2024. It is now read-only.

Commit

Permalink
refactor(advice): remove adviseall field
Browse files Browse the repository at this point in the history
Also no longer implement a special getproperty method. Instead we can
just call a dedicated function when appropriate.
  • Loading branch information
tecosaur committed May 26, 2024
1 parent c3da8e5 commit 754b580
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 35 deletions.
56 changes: 34 additions & 22 deletions src/model/advice.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -56,28 +56,35 @@ 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)
"""
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)
nothing
end
end
getfield(dta, prop)
end

AdviceAmalgamation(plugins::Vector{String}) =
AdviceAmalgamation(identity, Advice[], plugins, String[])
AdviceAmalgamation(Advice[], plugins, String[])

AdviceAmalgamation(collection::DataCollection) =
AdviceAmalgamation(collection.plugins)
Expand All @@ -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
Expand Down
4 changes: 1 addition & 3 deletions src/model/types.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
15 changes: 5 additions & 10 deletions test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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[]
Expand All @@ -117,7 +112,7 @@ 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]
Expand Down

0 comments on commit 754b580

Please sign in to comment.