search

JuliaAI / DecisionTree.jl

Julia implementation of Decision Tree (CART) and Random Forest algorithms
Other
342 stars 101 forks source link

Fix type piracy on `zero` #186

Closed rikhuijzer closed 2 years ago

rikhuijzer commented 2 years ago

This fixes the type piracy on zero. In #182, @yufongpeng has correctly changed

zero(String) = ""

to

zero(::Type{String}) = ""

because with the old implementation in DecisionTree 0.10, the following happens

julia> using Statistics

julia> mean([[1, 2]]; dims=1)
1-element Vector{Vector{Float64}}:
 [1.0, 2.0]

julia> using DecisionTree

julia> mean([[1, 2]]; dims=1)
ERROR: MethodError: no method matching +(::String, ::String)
[...]

since

julia> zero(String) = ""

julia> zero(1)
""

which used to be a very bad type piracy which was luckily solved already by Yu-Fong, Peng.

However, it's still a type piracy because String is not owned by DecisionTree.

Let's go for some extra safety by adding an intermediate function.

codecov-commenter commented 2 years ago

Codecov Report

Merging #186 (1595ad5) into dev (2e5be13) will increase coverage by 0.00%. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##              dev     #186   +/-   ##
=======================================
  Coverage   89.55%   89.56%           
=======================================
  Files          10       10           
  Lines        1178     1179    +1     
=======================================
+ Hits         1055     1056    +1     
  Misses        123      123
Impacted Files Coverage Δ
src/DecisionTree.jl 52.23% <100.00%> (+0.72%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 2e5be13...1595ad5. Read the comment docs.