From 281c2c8bf4e74e7db8fddd42e8fa4b070cfcf32d Mon Sep 17 00:00:00 2001 From: "Anthony D. Blaom" Date: Fri, 5 Jan 2024 12:18:09 +1300 Subject: [PATCH 1/3] add Aqua.jl to tests and address issues raised --- Project.toml | 16 ++++++++++-- src/data_utils.jl | 6 ++--- src/equality.jl | 5 ++-- src/model_traits.jl | 59 +++++++++++++++++------------------------- test/aqua.jl | 4 +++ test/data_utils.jl | 27 +++++++++---------- test/metadata_utils.jl | 6 ----- test/model_traits.jl | 12 ++------- test/runtests.jl | 43 ++++++++++++++++++++++++------ 9 files changed, 99 insertions(+), 79 deletions(-) create mode 100644 test/aqua.jl diff --git a/Project.toml b/Project.toml index 567119a..93546fd 100644 --- a/Project.toml +++ b/Project.toml @@ -9,11 +9,23 @@ ScientificTypesBase = "30f210dd-8aff-4c5f-94ba-8e64358c1161" StatisticalTraits = "64bff920-2084-43da-a3e6-9bb72801c0c9" [compat] -ScientificTypesBase = "3.0" +Aqua = "0.8" +CategoricalArrays = "0.10" +DataFrames = "1" +Distances = "0.10" +InteractiveUtils = "<0.0.1, 1" +Markdown = "<0.0.1, 1" +OrderedCollections = "1" +Random = "<0.0.1, 1" +ScientificTypes = "3" +ScientificTypesBase = "3" StatisticalTraits = "3.2" +Tables = "1" +Test = "<0.0.1, 1" julia = "1.6" [extras] +Aqua = "4c88cf16-eb10-579e-8560-4a9242c79595" CategoricalArrays = "324d7699-5711-5eae-9e2f-1d82baa6b597" DataFrames = "a93c6f00-e57d-5684-b7b6-d8193f3e46c0" Distances = "b4f34e82-e78d-54a5-968a-f98e89d6e8f7" @@ -25,4 +37,4 @@ Tables = "bd369af6-aec1-5ad0-b16a-f7cc5008161c" Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40" [targets] -test = ["CategoricalArrays", "DataFrames", "Distances", "InteractiveUtils", "Markdown", "OrderedCollections", "ScientificTypes", "Tables", "Test"] +test = ["Aqua", "CategoricalArrays", "DataFrames", "Distances", "InteractiveUtils", "Markdown", "OrderedCollections", "ScientificTypes", "Tables", "Test"] diff --git a/src/data_utils.jl b/src/data_utils.jl index cec794b..c598aa2 100644 --- a/src/data_utils.jl +++ b/src/data_utils.jl @@ -58,7 +58,7 @@ end # int """ - int(x; type=nothing) + int(x) The positional integer of the `CategoricalString` or `CategoricalValue` `x`, in the ordering defined by the pool of `x`. The type of `int(x)` is the reference @@ -96,9 +96,9 @@ julia> int(v) ``` See also: [`decoder`](@ref). """ -function int(x; type::Union{Nothing, Type{T}}=nothing) where T <: Real +function int(x; type=nothing) type === nothing && return int(get_interface_mode(), x) - return convert.(T, int(get_interface_mode(), x)) + return convert.(type, int(get_interface_mode(), x)) end int(::LightInterface, x) = errlight("int") diff --git a/src/equality.jl b/src/equality.jl index 83cd272..90502a1 100644 --- a/src/equality.jl +++ b/src/equality.jl @@ -124,7 +124,7 @@ function is_same_except(m1::M1, is_same_except( getproperty(m1, name), getproperty(m2, name) - ) || + ) || getproperty(m1, name) isa AbstractRNG || getproperty(m2, name) isa AbstractRNG ) || return false @@ -155,7 +155,8 @@ function special_in(x, itr)::Union{Bool,Missing} end Base.in(x::MLJType, itr::Set) = special_in(x, itr) -Base.in(x::MLJType, itr::AbstractVector) = special_in(x, itr) +Base.in(x::MLJType, itr::AbstractVector{<:MLJType}) = special_in(x, itr) +Base.in(x::MLJType, itr::AbstractRange{<:MLJType}) = special_in(x, itr) Base.in(x::MLJType, itr::Tuple) = special_in(x, itr) # A version of `in` that actually uses `==`: diff --git a/src/model_traits.jl b/src/model_traits.jl index 3557283..7f30d27 100644 --- a/src/model_traits.jl +++ b/src/model_traits.jl @@ -97,50 +97,39 @@ end # in `prediction_type` for models which, historically, have not # implemented the trait. +# Actually, this has proven more trouble than it's worth and should be removed in 2.0. +# See https://discourse.julialang.org/t/deconstructing-unionall-types/108328 to appreciate +# some of the complications. + function StatTraits.predict_scitype( M::Type{<:Union{Probabilistic, ProbabilisticDetector}} ) return _density(target_scitype(M)) end -_density(::Any) = Unknown - -for T in [:Continuous, :Count, :Textual] - eval( - quote - function _density(::Type{AbstractArray{$T, D}}) where D - return AbstractArray{Density{$T}, D} - end - end - ) -end +const SCALAR_SCITYPES_EXS = + [:Finite, :Multiclass, :OrderedFactor, :Infinite, :Continuous, :Count, :Textual] -for T in [:Finite, :Multiclass, :OrderedFactor, :Infinite, :Continuous, :Count, :Textual] - eval( - quote - function _density(::Type{AbstractArray{<:$T, D}}) where D - return AbstractArray{Density{<:$T}, D} - end +const SCALAR_SCITYPES = + eval.([:Finite, :Multiclass, :OrderedFactor, :Infinite, :Continuous, :Count, :Textual]) - _density(::Type{Table($T)}) = Table(Density{$T}) - end - ) +function _density(t) + for T in SCALAR_SCITYPES + t == AbstractVector{<:T} && return AbstractVector{Density{<:T}} + t == AbstractMatrix{<:T} && return AbstractMatrix{Density{<:T}} + t == Table(T) && return Table{<:AbstractVector{<:Density{<:T}}} + end + for T in [Finite, Multiclass, OrderedFactor] + t == Table(T{2}) && return Table(Density{<:T{2}}) + end + return Unknown end - -for T in [:Finite, :Multiclass, :OrderedFactor] - eval( +for T in SCALAR_SCITYPES_EXS quote - function _density(::Type{AbstractArray{<:$T{N}, D}}) where {N, D} - return AbstractArray{Density{<:$T{N}}, D} - end - - function _density(::Type{AbstractArray{$T{N}, D}}) where {N, D} - return AbstractArray{Density{$T{N}}, D} - end - - _density(::Type{Table($T{N})}) where N = Table(Density{$T{N}}) - end - ) + _density(::Type{<:AbstractArray{W, D}}) where {W<:$T, D} = + AbstractArray{Density{W}, D} + _density(::Type{<:Table{<:AbstractVector{W}}}) where W<:$T = + Table(Density{W}) + end |> eval end - diff --git a/test/aqua.jl b/test/aqua.jl new file mode 100644 index 0000000..a02050d --- /dev/null +++ b/test/aqua.jl @@ -0,0 +1,4 @@ +import Aqua +import MLJModelInterface + +Aqua.test_all(MLJModelInterface, ambiguities=true) diff --git a/test/data_utils.jl b/test/data_utils.jl index 6c1eb75..95d1024 100644 --- a/test/data_utils.jl +++ b/test/data_utils.jl @@ -23,14 +23,15 @@ end # needing the `FullInterface`. X = (1, 2, 3) @test_throws M.InterfaceError matrix(X) - + X = (a=[1, 2, 3], b=[1, 2, 3]) @test_throws M.InterfaceError matrix(X) end @testset "matrix-full" begin setfull() - M.matrix(::FI, ::Val{:table}, X; kw...) = Tables.matrix(X; kw...) + # next line commented out as already defined in test/mode.jl: + # M.matrix(::FI, ::Val{:table}, X; kw...) = Tables.matrix(X; kw...) X = (a=[1, 2, 3], b=[1, 2, 3]) @test matrix(X) == hcat([1, 2, 3], [1, 2, 3]) end @@ -120,14 +121,14 @@ end # ------------------------------------------------------------------------ @testset "istable" begin # Nothing stops someone from implementing a Tables.jl - # interface that subtypes `AbstractArray`, so therefore + # interface that subtypes `AbstractArray`, so therefore # `istable` should throw an error for `LightInterface` setlight() X = rand(5) @test_throws M.InterfaceError M.istable(X) X = randn(5, 5) @test_throws M.InterfaceError M.istable(X) - + # The method runs in `FullInterface` setfull() X = rand(5) @@ -184,10 +185,10 @@ end X = ones(5) @test nrows(X) == 5 X = ones(5, 3) - @test nrows(X) == 5 - # It doesn't make sense to get the numbers of rows for - # `AbstractArray`'s of dimension 3 or more. Except if these are - # defined as Tables. Hence `FullInterface` would be required to check this + @test nrows(X) == 5 + # It doesn't make sense to get the numbers of rows for + # `AbstractArray`'s of dimension 3 or more. Except if these are + # defined as Tables. Hence `FullInterface` would be required to check this X = ones(5, 3, 2) @test_throws ArgumentError nrows(X) M.nrows(::FI, ::Val{:table}, X) = Tables.rowcount(X) @@ -220,13 +221,13 @@ end @testset "select-full" begin setfull() - + # test fallback X = nothing @test selectrows(X, 1) === nothing @test selectcols(X, 1) === nothing @test select(X, 1, 2) === nothing - + # vector X = ones(5) @test selectrows(X, 1) == [1.0] @@ -273,11 +274,11 @@ end project(t::NamedTuple, label::Colon) = t project(t::NamedTuple, label::Symbol) = project(t, [label,]) - + function project(t::NamedTuple, indices::AbstractArray{<:Integer}) return NamedTuple{tuple(keys(t)[indices]...)}(tuple([t[i] for i in indices]...)) end - + project(t::NamedTuple, i::Integer) = project(t, [i,]) X = (x=[1, 2, 3], y=[4, 5, 6], z=[0, 0, 0]) @@ -292,7 +293,7 @@ end @test select(X, :, 1) == [1, 2, 3] @test selectcols(X, :x) == [1, 2, 3] @test select(X, 1:2, :z) == [0, 0] - + # extra tests by Anthony X = (x=[1, 2, 3], y=[10, 20, 30], z= [:a, :b, :c]) @test select(X, 2, :y) == 20 diff --git a/test/metadata_utils.jl b/test/metadata_utils.jl index cd06ce4..471ef65 100644 --- a/test/metadata_utils.jl +++ b/test/metadata_utils.jl @@ -198,12 +198,6 @@ metadata_pkg(FooRegressor2, is_wrapper=false ) -# this is added in MLJBase but not in MLJModelInterface, to avoid -# InteractiveUtils as dependency: -setfull() -M.implemented_methods(::FI, M::Type{<:MLJType}) = - getfield.(methodswith(M), :name) - const HEADER2 = MLJModelInterface.doc_header(FooRegressor2, augment=true) @doc """ diff --git a/test/model_traits.jl b/test/model_traits.jl index 5b08fb5..f5f52e1 100644 --- a/test/model_traits.jl +++ b/test/model_traits.jl @@ -114,10 +114,6 @@ M.human_name(::Type{<:U1}) = "funky model" setfull() - function M.implemented_methods(::FI, M::Type{<:MLJType}) - return getfield.(methodswith(M), :name) - end - @test Set(implemented_methods(mp)) == Set([:clean!,:bar,:foo]) end @@ -140,19 +136,15 @@ end M._density(AbstractVector{<:T}), AbstractVector{Density{<:T}} ) - @test M._density(Table(T)) == Table(Density{T}) + @test M._density(Table(T)) == Table(Density{<:T}) end for T in [Finite, Multiclass, OrderedFactor] - @test ==( - M._density(AbstractArray{<:T{2},3}), - AbstractArray{Density{<:T{2}},3} - ) @test ==( M._density(AbstractArray{T{2},3}), AbstractArray{Density{T{2}},3} ) - @test M._density(Table(T{2})) == Table(Density{T{2}}) + @test M._density(Table(T{2})) == Table(Density{<:T{2}}) end end diff --git a/test/runtests.jl b/test/runtests.jl index 693f2da..59ad5d7 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -4,6 +4,7 @@ using Tables, Distances, CategoricalArrays, InteractiveUtils import DataFrames: DataFrame import Markdown import OrderedCollections +import Aqua const M = MLJModelInterface const FI = M.FullInterface @@ -11,11 +12,37 @@ const FI = M.FullInterface setlight() = M.set_interface_mode(M.LightInterface()) setfull() = M.set_interface_mode(M.FullInterface()) -include("mode.jl") -include("parameter_inspection.jl") -include("data_utils.jl") -include("metadata_utils.jl") -include("model_def.jl") -include("model_api.jl") -include("model_traits.jl") -include("equality.jl") +@testset "mode.jl" begin + include("mode.jl") +end + +@testset "parameter_inspection.jl" begin + include("parameter_inspection.jl") +end + +@testset "data_utils.jl" begin + include("data_utils.jl") +end + +@testset "metadata_utils.jl" begin + include("metadata_utils.jl") +end +@testset "model_def.jl" begin + include("model_def.jl") +end + +@testset "model_api.jl" begin + include("model_api.jl") +end + +@testset "model_traits.jl" begin + include("model_traits.jl") +end + +@testset "equality.jl" begin + include("equality.jl") +end + +@testset "aqua.jl" begin + include("aqua.jl") +end From 4ac5584e4d3bc2c2eaf1d6a20dff8f31767a3e1c Mon Sep 17 00:00:00 2001 From: "Anthony D. Blaom" Date: Thu, 11 Jan 2024 12:47:56 +1300 Subject: [PATCH 2/3] update codecov --- .github/codecov.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/codecov.yml b/.github/codecov.yml index dda9ad0..ed9d9f1 100644 --- a/.github/codecov.yml +++ b/.github/codecov.yml @@ -2,4 +2,7 @@ coverage: status: project: default: - threshold: 0.5% \ No newline at end of file + threshold: 0.5% + patch: + default: + target: 80% \ No newline at end of file From 7578027ee6379af4dbd52fd85fa7a229c1c1619a Mon Sep 17 00:00:00 2001 From: "Anthony D. Blaom" Date: Thu, 11 Jan 2024 15:09:13 +1300 Subject: [PATCH 3/3] bump 1.9.5 --- Project.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Project.toml b/Project.toml index 93546fd..2fa4b2e 100644 --- a/Project.toml +++ b/Project.toml @@ -1,7 +1,7 @@ name = "MLJModelInterface" uuid = "e80e1ace-859a-464e-9ed9-23947d8ae3ea" authors = ["Thibaut Lienart and Anthony Blaom"] -version = "1.9.4" +version = "1.9.5" [deps] Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"