add Aqua.jl tests and refactor code

Author: OkonSamuel

Project.toml

@@ -9,17 +9,24 @@ ScientificTypesBase = "30f210dd-8aff-4c5f-94ba-8e64358c1161"
 Tables = "bd369af6-aec1-5ad0-b16a-f7cc5008161c"
 
 [compat]
-julia = "1"
+Aqua = "0.8"
+Distributions = "0.25"
+julia = "1.6"
+MLJBase = "1.1"
+MLJDecisionTreeInterface = "0.4"
 MLJModelInterface = "1.4"
 ScientificTypesBase = "3"
+StableRNGs = "1"
 Tables = "1.2"
+Test = "1.6"
 
 [extras]
+Aqua = "4c88cf16-eb10-579e-8560-4a9242c79595"
 Distributions = "31c24e10-a181-5473-b8eb-7969acd0382f"
 MLJBase = "a7f614a8-145f-11e9-1d2a-a57a1082229d"
 MLJDecisionTreeInterface = "c6f25543-311c-4c74-83dc-3ea6d1015661"
 StableRNGs = "860ef19b-820b-49d6-a774-d7a799459cd3"
 Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
 
 [targets]
-test = ["Distributions", "MLJBase", "MLJDecisionTreeInterface", "StableRNGs", "Test"]
+test = ["Aqua", "Distributions", "MLJBase", "MLJDecisionTreeInterface", "StableRNGs", "Test"]

README.md

@@ -24,7 +24,7 @@ known in the R community as the friedman dataset#1. Notice how the target vector
 dataset depends on only the first five columns of feature table. So we expect that our 
 recursive feature elimination should return the first columns as important features.
 ```julia
-using MLJ # or, minimally, `using FeatureSelection, MLJModels, MLJBase`
+using MLJ, FeatureSelection
 using StableRNGs
 rng = StableRNG(123)
 A = rand(rng, 50, 10)
@@ -41,20 +41,26 @@ train it on our dataset
 RandomForestRegressor = @load RandomForestRegressor pkg=DecisionTree
 forest = RandomForestRegressor()
 rfe = RecursiveFeatureElimination(
-    model = forest, n_features_to_select=5, step=1
+    model = forest, n_features=5, step=1
 ) # see doctring for description of defaults
 mach = machine(rfe, X, y)
 fit!(mach)
 ```
-We can view the important features by inspecting the `fitted_params` object.
+If we wish, we can get the feature importance scores, either by inspecting `report(mach)` 
+or calling the `feature_importances` function on the fitted machine as shown below
+```julia
+report(mach).ranking # returns [1.0, 1.0, 1.0, 1.0, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0]
+feature_importances(mach) # returns dict of feature => rank pairs
+```
+We can view the important features used by our model by inspecting the `fitted_params` object.
 ```julia
 p = fitted_params(mach)
 p.features_left == [:x1, :x2, :x3, :x4, :x5]
 ```
-We can also call `predict` on the fitted machine, to predict using a 
-random forest regressor trained just on those features, or call `transform`, to 
-select just those features some new table including all the original features.
-in `?RecursiveFeatureElimination`.
+We can also call the `predict` method on the fitted machine, to predict using a 
+random forest regressor trained using only the important features, or call the `transform` 
+method, to select just those features from some new table including all the original features.
+For more info, type `?RecursiveFeatureElimination` on a Julia REPL.
 
 Okay, let's say that we didn't know that our synthetic dataset depends on only five 
 columns from our feature table. We could apply cross fold validation `CV(nfolds=5)` with 
@@ -66,18 +72,18 @@ rfe = RecursiveFeatureElimination(model = forest)
 tuning_rfe_model  = TunedModel(
     model = rfe,
     measure = rms,
-    tuning = Grid(rng=rng, resolution=10),
-    resampling = CV(nfolds = 5),
+    tuning = Grid(rng=rng),
+    resampling = StratifiedCV(nfolds = 5),
     range = range(
-        rfe, :n_features_to_select, values = collect(2:8)
+        rfe, :n_features, lower = 1, upper=10, unit=1
     )
 )
 self_tuning_rfe_mach = machine(tuning_rfe_model, X, y)
 fit!(self_tuning_rfe_mach)
 ```
-As before we can inspect the important features by inspesting the `fitted_params` object.
+As before we can inspect the important features by inspecting the `fitted_params` object.
 ```julia
-fitted_parms(self_tuning_rfe_mach).best_model.features_left == [:x1, :x2, :x3, :x4, :x5]
+fitted_params(self_tuning_rfe_mach).best_fitted_params.features_left == [:x1, :x2, :x3, :x4, :x5]
 ```
 and call `predict` on the tuned model machine as shown below
 ```julia

src/FeatureSelection.jl

@@ -1,6 +1,6 @@
 module FeatureSelection
 
-using MLJModelInterface, Tables, ScientificTypesBase, MLJTuning
+using MLJModelInterface, Tables, ScientificTypesBase
 
 export FeatureSelector, RecursiveFeatureElimination
 

src/models/featureselector.jl

@@ -84,7 +84,7 @@ MMI.metadata_model(
     FeatureSelector,
     input_scitype = Table,
     output_scitype = Table,
-    load_path = "MLJModels.FeatureSelector"
+    load_path = "FeatureSelction.FeatureSelector"
 )
 
 """

src/models/rfe.jl

@@ -11,11 +11,17 @@ const ERR_MODEL_TYPE = ArgumentError(
 )
 
 const ERR_FEATURE_IMPORTANCE_SUPPORT = ArgumentError(
-        "Model does not report feature importance, hence recursive feature algorithm "*
-        "can't be applied."
+    "Model does not report feature importance, hence recursive feature algorithm "*
+    "can't be applied."
 )
 
-const MODEL_TYPES = [:ProbabilisticRecursiveFeatureElimination, :DeterministicRecursiveFeatureElimination]
+const ERR_FEATURES_SEEN = ArgumentError(
+    "Features of new table must be same as those seen during fit process."
+)
+
+const MODEL_TYPES = [
+    :ProbabilisticRecursiveFeatureElimination, :DeterministicRecursiveFeatureElimination
+]
 const SUPER_TYPES = [:Deterministic, :Probabilistic]
 const MODELTYPE_GIVEN_SUPERTYPES = zip(MODEL_TYPES, SUPER_TYPES)
 
@@ -114,7 +120,9 @@ RandomForestRegressor = @load RandomForestRegressor pkg=DecisionTree
 
 # Creates a dataset where the target only depends on the first 5 columns of the input table.
 A = rand(rng, 50, 10);
-y = 10 .* sin.(pi .* A[:, 1] .* A[:, 2]) + 20 .* (A[:, 3] .- 0.5).^ 2 .+ 10 .* A[:, 4] .+ 5 * A[:, 5]);
+y = 10 .* sin.(
+        pi .* A[:, 1] .* A[:, 2]
+    ) + 20 .* (A[:, 3] .- 0.5).^ 2 .+ 10 .* A[:, 4] .+ 5 * A[:, 5]);
 X = MLJ.table(A);
 
 # fit a rfe model
@@ -189,7 +197,9 @@ function MMI.fit(selector::RFE, verbosity::Int, X, y, args...)
     Xcols = Tables.Columns(X)
     features = collect(Tables.columnnames(Xcols))
     nfeatures = length(features)
-    nfeatures < 2 && throw(ArgumentError("The number of features in the feature matrix must be at least 2."))
+    nfeatures < 2 && throw(
+        ArgumentError("The number of features in the feature matrix must be at least 2.")
+    )
 
     # Compute required number of features to select
     n_features = selector.n_features # Remember to modify this estimate later
@@ -256,12 +266,12 @@ function MMI.fit(selector::RFE, verbosity::Int, X, y, args...)
     fitresult = (
         support = support,
         model_fitresult = model_fitresult,
-        features_left = copy(features_left)
+        features_left = copy(features_left),
+        features = features
     )
     report = ( 
         ranking = ranking,
-        model_report = model_report,
-        features = features
+        model_report = model_report
     )
 
     return fitresult, nothing, report
@@ -282,20 +292,27 @@ function MMI.predict(model::RFE, fitresult, X)
 end
 
 function MMI.transform(::RFE, fitresult, X)
+    sch = Tables.schema(Tables.columns(X))
+    if (length(fitresult.features) == length(sch.names) && 
+        !all(e -> e in sch.names, fitresult.features))
+        throw(
+            ERR_FEATURES_SEEN
+        )
+    end
     return MMI.selectcols(X, fitresult.features_left)
 end
 
 function MMI.feature_importances(::RFE, fitresult, report)
-    return Pair.(report.features, report.ranking)
+    return Pair.(fitresult.features, report.ranking)
 end
 
 ## Traits definitions
 function MMI.load_path(::Type{<:DeterministicRecursiveFeatureElimination})
-    return "FeatureEngineering.DeterministicRecursiveFeatureElimination"
+    return "FeatureSelection.DeterministicRecursiveFeatureElimination"
 end
 
 function MMI.load_path(::Type{<:ProbabilisticRecursiveFeatureElimination})
-    return "FeatureEngineering.ProbabilisticRecursiveFeatureElimination"
+    return "FeatureSelection.ProbabilisticRecursiveFeatureElimination"
 end
 
 for trait in [
@@ -323,13 +340,17 @@ end
 
 # ## Iteration parameter
 # at level of types:
+prepend(s::Symbol, ::Nothing) = nothing
+prepend(s::Symbol, t::Symbol) = Expr(:(.), s, QuoteNode(t))
+prepend(s::Symbol, ex::Expr) = Expr(:(.), prepend(s, ex.args[1]), ex.args[2])
+
 function MMI.iteration_parameter(::Type{<:RFE{M}}) where {M}
-    return MLJModels.prepend(:model, MMI.iteration_parameter(M))
+    return prepend(:model, MMI.iteration_parameter(M))
 end
 
 # at level of instances:
 function MMI.iteration_parameter(model::RFE)
-    return MLJModels.prepend(:model, MMI.iteration_parameter(model.model))
+    return prepend(:model, MMI.iteration_parameter(model.model))
 end
 
 ## TRAINING LOSSES SUPPORT

test/Aqua.jl

@@ -0,0 +1,5 @@
+using Aqua
+
+@testset "Aqua.jl" begin
+    Aqua.test_all(FeatureSelection) 
+end

test/models/dummy_test_models.jl

@@ -0,0 +1,25 @@
+module DummyTestModels
+
+using MLJBase
+
+## THE CONSTANT DETERMINISTIC REGRESSOR (FOR TESTING)
+##
+
+struct DeterministicConstantRegressor <: MLJBase.Deterministic end
+
+function MLJBase.fit(::DeterministicConstantRegressor, verbosity::Int, X, y)
+    fitresult = mean(y)
+    cache     = nothing
+    report    = nothing
+    return fitresult, cache, report
+end
+
+MLJBase.reformat(::DeterministicConstantRegressor, X) = (MLJBase.matrix(X),)
+MLJBase.reformat(::DeterministicConstantRegressor, X, y) = (MLJBase.matrix(X), y)
+MLJBase.selectrows(::DeterministicConstantRegressor, I, A) = (view(A, I, :),)
+MLJBase.selectrows(::DeterministicConstantRegressor, I, A, y) =
+    (view(A, I, :), y[I])
+
+MLJBase.predict(::DeterministicConstantRegressor, fitresult, Xnew) =
+    fill(fitresult, nrows(Xnew))
+end

test/models/featureselector.jl

@@ -0,0 +1,70 @@
+#### FEATURE SELECTOR ####
+
+@testset "Feat Selector" begin
+    N = 100
+    X = (
+        Zn   = rand(N),
+        Crim = rand(N),
+        x3   = categorical(rand("YN", N)),
+        x4   = categorical(rand("YN", N))
+    )
+
+    # Test feature selection with `features=Symbol[]`
+    namesX   = MLJBase.schema(X).names |> collect
+    selector = FeatureSelector()
+    f,       = MLJBase.fit(selector, 1, X)
+    @test f == namesX
+    Xt = MLJBase.transform(selector, f, MLJBase.selectrows(X, 1:2))
+    @test Set(MLJBase.schema(Xt).names) == Set(namesX)
+    @test length(Xt.Zn) == 2
+
+    # Test on selecting features if `features` keyword is defined
+    selector = FeatureSelector(features=[:Zn, :Crim])
+    f,       = MLJBase.fit(selector, 1, X)
+    @test MLJBase.transform(selector, f, MLJBase.selectrows(X, 1:2)) ==
+            MLJBase.select(X, 1:2, [:Zn, :Crim])
+
+    # test on ignoring a feature, even if it's listed in the `features`
+    selector.ignore = true
+    f,   = MLJBase.fit(selector, 1, X)
+    Xnew = MLJBase.transform(selector, f, X)
+    @test MLJBase.transform(selector, f, MLJBase.selectrows(X, 1:2)) ==
+         MLJBase.select(X, 1:2, [:x3, :x4])
+
+    # test error about features selected or excluded in fit.
+    selector = FeatureSelector(features=[:x1, :mickey_mouse])
+    @test_throws(
+        ArgumentError,
+        MLJBase.fit(selector, 1, X)
+    )
+    selector.ignore = true
+    @test_logs(
+        (:warn, r"Excluding non-existent"),
+        MLJBase.fit(selector, 1, X)
+    )
+
+    # features must be specified if ignore=true
+    @test_throws ArgumentError FeatureSelector(ignore=true)
+
+    # test logs for no features selected when using Bool-Callable function interface:
+    selector = FeatureSelector(features= x-> x == (:x1))
+   @test_throws(
+        ArgumentError,
+        MLJBase.fit(selector, 1, X)
+    )
+    selector.ignore = true
+    selector.features = x-> x in [:Zn, :Crim, :x3, :x4]
+     @test_throws(
+        ArgumentError,
+        MLJBase.fit(selector, 1, X)
+    )
+
+    # Test model Metadata
+    @test MLJBase.input_scitype(selector) == MLJBase.Table
+    @test MLJBase.output_scitype(selector) == MLJBase.Table
+end
+
+#  To be added with FeatureSelectorRule X = (n1=["a", "b", "a"], n2=["g", "g", "g"], n3=[7, 8, 9],
+#               n4 =UInt8[3,5,10],  o1=[4.5, 3.6, 4.0], )
+# MLJBase.schema(X)
+# Xc = coerce(X,  :n1=>Multiclass, :n2=>Multiclass)

test/models/rfe.jl

@@ -0,0 +1,56 @@
+using .DummyTestModels
+const DTM = DummyTestModels
+
+@testset "RecursiveFeatureElimination" begin
+    #@test_throws ArgumentError RecursiveFeatureElimination(model = rf)
+    # Data For use in testset
+    X = rand(rng, 50, 10)
+    y = @views(
+        10 .* sin.(
+            pi .* X[:, 1] .* X[:, 2]
+        ) + 20 .* (X[:, 3] .- 0.5).^ 2 .+ 10 .* X[:, 4] .+ 5 * X[:, 5]
+    )
+    Xt = MLJBase.table(X)
+    Xnew = MLJBase.table(rand(rng, 50, 10))
+    Xnew2 = MLJBase.table(rand(rng, 50, 10), names = [Symbol("y$i") for i in 1:10])
+
+    # Constructor
+    @test_throws FeatureSelection.ERR_SPECIFY_MODEL RecursiveFeatureElimination()
+    reg = DTM.DeterministicConstantRegressor()
+    @test_throws(
+        FeatureSelection.ERR_FEATURE_IMPORTANCE_SUPPORT, 
+        RecursiveFeatureElimination(model = DTM.DeterministicConstantRegressor())
+    )
+    rf = RandomForestRegressor()
+    selector = RecursiveFeatureElimination(model = rf)
+    @test selector isa FeatureSelection.DeterministicRecursiveFeatureElimination
+
+    # Fit
+    selector_mach = machine(selector, Xt, y)
+    fit!(selector_mach)
+    selector_fp = fitted_params(selector_mach)
+    @test propertynames(selector_fp) == (:features_left, :model_fitresult)
+    @test selector_fp.features_left == [:x1, :x2, :x3, :x4, :x5]
+    @test selector_fp.model_fitresult == MLJBase.fitted_params(
+        selector_mach.model.model, selector_mach.fitresult.model_fitresult
+    )
+    @test feature_importances(selector_mach) == [
+        :x1 => 1.0, :x2 => 1.0, :x3 => 1.0, :x4 => 1.0, :x5 => 1.0, 
+        :x6 => 2.0, :x7 => 3.0, :x8 => 4.0, :x9 => 5.0, :x10 => 6.0
+    ]
+    rpt = report(selector_mach)
+    @test rpt.ranking == [
+        1.0, 1.0, 1.0, 1.0, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0
+    ]
+
+    # predict
+    yhat = predict(selector_mach, Xnew)
+    @test scitype(yhat) === AbstractVector{Continuous}
+
+    # transform
+    trf = transform(selector_mach, Xnew)
+    sch = MLJBase.schema(trf)
+    @test sch.names === (:x1, :x2, :x3, :x4, :x5)
+    @test sch.scitypes === (Continuous, Continuous, Continuous, Continuous, Continuous)
+    @test_throws FeatureSelection.ERR_FEATURES_SEEN transform(selector_mach, Xnew2)
+end