Added Tests and found bugs to be fixed in src/ #68
Conversation
|
This PR does not pass the CI tests. It should not have been merged. We should revert and fix the errors before merging. |
| @test srs.data.probs == 1 ./ srs.data.weights | ||
|
|
||
| srs_weights = SimpleRandomSample(apisrs, ignorefpc = false, weights = :fpc) | ||
| @test_throws srs_weights = SimpleRandomSample(apisrs, ignorefpc = false, weights = :stype) |
There was a problem hiding this comment.
The @test_throws macro needs another argument, which is the error type. See the documentation. In this case the test throws a MethodError because of a lateral error (not because weights cannot be a string vector, but because the operations done on weights give errors for strings), which is fine for now but eventually it should throw a more specific error.
| srs_w_p = SimpleRandomSample(apisrs, ignorefpc = false, weights = :fpc, probs = fill(0.3, size(apisrs_original, 1))) | ||
|
|
||
| srs = SimpleRandomSample(apisrs, ignorefpc = true, probs = 1 ./ apisrs.pw ) | ||
| @test srs.data.probs == 1 ./ srs.data.weights |
There was a problem hiding this comment.
This test fails and it should indeed fail because, I believe the probability weights should be frequency weights divided by population size. In this case, probs should be given apisrs.pw ./ apisrs.fpc and we should check that srs.data.weights == srs.data.probs .* srs.popsize.
There was a problem hiding this comment.
Also, remove the space before the last paranthesis on line 29.
| @@ -1,19 +1,30 @@ | |||
| @testset "svymean.jl" begin | |||
| # SimpleRandomSample | |||
| using DataFrames | |||
There was a problem hiding this comment.
This line should not be here. If you need to use DataFrames functionality use DataFrames. as prefix.
|
|
||
| # StratifiedSample | ||
| srs_w_p = srs_weights = SimpleRandomSample(apisrs, ignorefpc = false, weights = :fpc, probs = fill(0.3, size(apisrs_original, 1))) |
There was a problem hiding this comment.
apisrs_original is undefined.
| srs = SimpleRandomSample(apisrs, ignorefpc = true, probs = 1 ./ apisrs.pw ) | ||
| @test srs.data.probs == 1 ./ srs.data.weights | ||
| srs = SimpleRandomSample(apisrs, popsize = -2.8, ignorefpc = true)# the errror is wrong | ||
| srs = SimpleRandomSample(apisrs, sampsize = -2.8, ignorefpc = true)# the function is working upto line 55 |
There was a problem hiding this comment.
These last two lines give errors. There are no tests attached to them. We should check that these lines do indeed throw an error with @test_throws ERROR_NAME. Otherwise, they will simply throw errors (uncaught) and stop the process. If the error thrown right now is wrong, maybe we should wait to do the merge after we fix the error.
Suggesstion: We can proceed by checking whether the popsize or the sampsize is nonnegative integer.