Skip to content

Commit

Permalink
Removed redundant/unreachable code and added more tests.
Browse files Browse the repository at this point in the history
  • Loading branch information
rofinn committed Oct 9, 2020
1 parent af45737 commit f224dc8
Show file tree
Hide file tree
Showing 8 changed files with 38 additions and 17 deletions.
5 changes: 0 additions & 5 deletions src/deprecated.jl
Original file line number Diff line number Diff line change
Expand Up @@ -228,11 +228,6 @@ function impute!(data::AbstractMatrix{Union{T, Missing}}, imp::Union{DropObs, Dr
return data
end

function impute!(data::AbstractVector{Union{T, Missing}}, imp::Union{DropObs, DropVars}) where T
data = impute(data, imp)
return data
end

impute!(data, imp::Union{DropObs, DropVars}) = impute(data, imp)

@deprecate impute(data, C::Chain) run(data, C) false
Expand Down
1 change: 1 addition & 0 deletions src/imputors/interp.jl
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ julia> impute(M, Interpolate(); dims=:rows)
struct Interpolate <: Imputor end

function _impute!(data::AbstractArray{<:Union{T, Missing}}, imp::Interpolate) where T
@assert !all(ismissing, data)
i = findfirst(!ismissing, data) + 1

while i < lastindex(data)
Expand Down
8 changes: 2 additions & 6 deletions src/imputors/locf.jl
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,9 @@ julia> impute(M, LOCF(); dims=:rows)
struct LOCF <: Imputor end

function _impute!(data::AbstractVector{Union{T, Missing}}, imp::LOCF) where T
start_idx = findfirst(!ismissing, data)
if start_idx === nothing
@debug "Cannot carry forward points when all values are missing"
return data
end
@assert !all(ismissing, data)
start_idx = findfirst(!ismissing, data) + 1

start_idx += 1
for i in start_idx:lastindex(data)
if ismissing(data[i])
data[i] = data[i-1]
Expand Down
8 changes: 2 additions & 6 deletions src/imputors/nocb.jl
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,9 @@ julia> impute(M, NOCB(); dims=:rows)
struct NOCB <: Imputor end

function _impute!(data::AbstractVector{Union{T, Missing}}, imp::NOCB) where T
end_idx = findlast(!ismissing, data)
if end_idx === nothing
@debug "Cannot carry backward points when all values are missing"
return data
end
@assert !all(ismissing, data)
end_idx = findlast(!ismissing, data) - 1

end_idx -= 1
for i in end_idx:-1:firstindex(data)
if ismissing(data[i])
data[i] = data[i+1]
Expand Down
15 changes: 15 additions & 0 deletions test/chain.jl
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
@testset "Chaining and Piping" begin
# TODO: Add tests at each section to double check that orig hasn't been overwritten.
orig = Impute.dataset("test/table/neuro") |> DataFrame

@testset "DataFrame" begin
Expand Down Expand Up @@ -122,4 +123,18 @@
# Confirm that we don't have any more missing values
@test all(!ismissing, result)
end

@testset "Multi-type" begin
data = Impute.dataset("test/table/neuro") |> Tables.matrix
@test any(ismissing, data)
# Filter out colunns with more than 400 missing values, Fill with 0, and check that
# everything was replaced
C = Impute.Filter(c -> count(ismissing, c) < 400) Impute.Replace(; values=0.0) Impute.Threshold()

result = Impute.run(data, C; dims=:cols)
@test size(result, 1) == size(data, 1)
# We should have filtered out 1 column
@test size(result, 2) < size(data, 2)
@test all(!ismissing, result)
end
end
14 changes: 14 additions & 0 deletions test/data.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
@testset "data" begin
datasets = Impute.datasets()

@testset "Impute.dataset($name)" for name in datasets
result = Impute.dataset(name)
if contains(name, "matrix")
@test isa(result, AbstractDict)
elseif contains(name, "table")
@test isa(result, CSV.File)
end
end

@test_throws ArgumentError Impute.dataset("foobar")
end
2 changes: 2 additions & 0 deletions test/imputors/svd.jl
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@
# Test having only missing data
c = missings(5, 2)
@test isequal(impute(c, tester.imp(; tester.kwargs...); dims=:cols), c)
c_ = tester.f!(deepcopy(c); dims=:cols)
@test isequal(c_, c)
end
end
# Internal `svd` call isn't supported by these type, but maybe they should be?
Expand Down
2 changes: 2 additions & 0 deletions test/testutils.jl
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,8 @@ function test_matrix(tester::ImputorTester)
# Test having only missing data
c = missings(5, 2)
@test isequal(impute(c, tester.imp(; tester.kwargs...); dims=:cols), c)
c_ = impute!(deepcopy(c), tester.imp(; tester.kwargs...); dims=:cols)
@test isequal(c_, c)
end
end
end
Expand Down

0 comments on commit f224dc8

Please sign in to comment.