search

JuliaStats / Statistics.jl

The Statistics stdlib that ships with Julia.
https://juliastats.org/Statistics.jl/dev/
Other
70 stars 40 forks source link

fix handling of Missing in cor #74

Closed bkamins closed 3 years ago

codecov[bot] commented 3 years ago

Codecov Report

Merging #74 (fad50b7) into master (862798b) will increase coverage by 97.67%. The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #74       +/-   ##
===========================================
+ Coverage    0.00%   97.67%   +97.67%     
===========================================
  Files           1        1               
  Lines         387      387               
===========================================
+ Hits            0      378      +378     
+ Misses        387        9      -378
Impacted Files Coverage Δ
src/Statistics.jl 97.67% <100.00%> (+97.67%) :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 862798b...fad50b7. Read the comment docs.

bkamins commented 3 years ago

So a tension in this PR that we have:

julia> using Statistics

julia> z = [1.0, im]
2-element Vector{ComplexF64}:
 1.0 + 0.0im
 0.0 + 1.0im

julia> var(z)
1.0

julia> cov(z, z)
1.0 + 0.0im

and we have a tension that the identity var(z) === cov(z, z) does not hold. But probably we should accept it.

nalimilan commented 3 years ago

Yeah, but AFAICT that's an expected result since the variance is guaranteed to be a real number, contrary to the covariance: https://en.wikipedia.org/wiki/Complex_random_variable#Variance_and_pseudo-variance https://selipot.github.io/talks/lecture2.pdf

Anyway var(z) == cov(z, z) holds so that's already quite consistent.

bkamins commented 3 years ago

an expected result since the variance is guaranteed to be a real number

Yes - that is why I said we should accept it.

Was this actually needed for CI to pass?

Ah - I added it to make the tests pass on 1.0. I will remove it