Deprecate varm and stdm
#736
Conversation
`varm` and `stdm` are redundant with `var(..., mean=m)` and `std(..., mean=m)`. Moreover, their status is not clear: they are technically exported by Statistics (but not by StatsBase itself), but their docstrings are not included in the manual. They were only tested indirectly, so add tests to ensure deprecations work. The status of `varm!` is even weirder as it's imported from Statistics, but Statistics does not export it, and it has no docstrings. Keep a deprecation just in case.
| @@ -39,3 +39,10 @@ end | |||
| ### Deprecated September 2019 | |||
| @deprecate sum(A::AbstractArray, w::AbstractWeights, dims::Int) sum(A, w, dims=dims) | |||
| @deprecate values(wv::AbstractWeights) convert(Vector, wv) | |||
|
|
|||
| ### Deprecated November 2021 | |||
| @deprecate stdm(x::RealArray, w::AbstractWeights, m::Real; corrected::DepBool=nothing) std(x, w, mean=m, corrected=corrected) false | |||
There was a problem hiding this comment.
why do we disable export? This is breaking I think
There was a problem hiding this comment.
No, because it's not exported currently. We still overload the function defined in Statistics though to it can be used after using Statistics.
There was a problem hiding this comment.
Ah - OK. I mixed it up with the fact that Statistics exports it.
| @@ -43,30 +23,22 @@ function var(v::RealArray, w::AbstractWeights; mean=nothing, | |||
| corrected = depcheck(:var, :corrected, corrected) | |||
|
|
|||
| if mean == nothing | |||
| varm(v, w, Statistics.mean(v, w); corrected=corrected) | |||
| _moment2(v, w, Statistics.mean(v, w); corrected=corrected) | |||
There was a problem hiding this comment.
why not var with kwarg? same below
There was a problem hiding this comment.
We're already in var. Calling ourselves with the computed mean would work but do you think it would really be cleaner?
(I'd like to deprecate _moment2 and similar functions at some point to simplify the API and implementation...)
| corrected=corrected) | ||
| elseif mean == nothing | ||
| varm!(R, A, w, Statistics.mean(A, w, dims=dims), dims; corrected=corrected) | ||
| mean = Base.reducedim_initarray(A, dims, 0, eltype(R)) |
There was a problem hiding this comment.
why is this not tested. Also the same as above - why is it better than just calling the default replacement?
There was a problem hiding this comment.
Turns out it's not tested on master already. This is an undocumented feature of var (which calls var!). I wish it wasn't allowed, but I guess we have keep supporting it now (and add tests for it)...
test/moments.jl
Outdated
| @@ -20,11 +20,13 @@ w = [3.84, 2.70, 8.29, 8.91, 9.71, 0.0] | |||
| @testset "Variance" begin | |||
| @test var(x, wv; corrected=false) ≈ expected_var | |||
| @test var(x, wv; mean=m, corrected=false) ≈ expected_var | |||
| @test varm(x, wv, m; corrected=false) ≈ expected_var | |||
There was a problem hiding this comment.
maybe we should make the code alignment consistent everywhere?
varmandstdmare redundant withvar(..., mean=m)andstd(..., mean=m). Moreover, their status is not clear: they are technically exported by Statistics (but not by StatsBase itself), but their docstrings are not included in the manual. They were only tested indirectly, so add tests to ensure deprecations work.The status of
varm!is even weirder as it's imported from Statistics, but Statistics does not export it, and it has no docstrings. Keep a deprecation just in case.