JuliaStats / Statistics.jl

The Statistics stdlib that ships with Julia.

https://juliastats.org/Statistics.jl/dev/

Other

72 stars 40 forks source link

Allow computing the correlation matrix from the covariance matrix only #75

Open ASaragga opened 3 years ago

ASaragga commented 3 years ago

Following discussions concerning issue JuliaStats/StatsBase.jl#652.

codecov[bot] commented 3 years ago

Codecov Report

Merging #75 (3c30462) into master (862798b) will not change coverage. The diff coverage is n/a.

@@          Coverage Diff           @@
##           master     #75   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files           1       1           
  Lines         387     455   +68     
======================================
- Misses        387     455   +68

Impacted Files	Coverage Δ
src/Statistics.jl	`0.00% <ø> (ø)`

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...3c30462. Read the comment docs.

nalimilan commented 3 years ago

Thanks. Can you add a test? Otherwise the function might break at any time without anybody noticing.

ASaragga commented 3 years ago

Certainly. Already tried the obvious @test cov2cor(cov1) ≈ cor1 @test cov2cor(cov2) ≈ cor2 but it throws an error...

On 19 Feb 2021, at 09:01, Milan Bouchet-Valat notifications@github.com wrote:

Thanks. Can you add a test? Otherwise the function might break at any time without anybody noticing.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/JuliaLang/Statistics.jl/pull/75#issuecomment-781935829, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALJ5RCBLRR3M5YZ4XZDIBEDS7YSGDANCNFSM4XWX3GUQ.

nalimilan commented 3 years ago

What kind of error?

ASaragga commented 3 years ago

The cov2cor test errors because it is not considering the method cov2cor!(::Array{Float64,2}) that was added to Statistics.jl. Sorry about this.

The error is

cov2cor: Error During Test at /home/runner/work/StatsBase.jl/StatsBase.jl/test/cov.jl:127 143 https://github.com/JuliaStats/StatsBase.jl/pull/662/checks?check_run_id=1942164575#step:6:143 Test threw exception 144 https://github.com/JuliaStats/StatsBase.jl/pull/662/checks?check_run_id=1942164575#step:6:144 Expression: cov2cor(cov1) ≈ cor1 145 https://github.com/JuliaStats/StatsBase.jl/pull/662/checks?check_run_id=1942164575#step:6:145 MethodError: no method matching cov2cor!(::Array{Float64,2})

On 19 Feb 2021, at 14:46, Milan Bouchet-Valat notifications@github.com wrote:

What kind of error?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/JuliaLang/Statistics.jl/pull/75#issuecomment-782119403, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALJ5RCAD53UUZJJCLELT52DS7Z2S7ANCNFSM4XWX3GUQ.

nalimilan commented 3 years ago

Well we're in Statistics.jl here, not in StatsBase. :-/

ASaragga commented 3 years ago

Indeed! Tests for the family of cov2cor methods in Statistics.jl appear to be missing?! Tried to follow some examples to keep the same style of tests. Sorry it is taking so long...

On 20 Feb 2021, at 14:47, Milan Bouchet-Valat notifications@github.com wrote:

Well we're in Statistics.jl here, not in StatsBase. :-/

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/JuliaLang/Statistics.jl/pull/75#issuecomment-782692657, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALJ5RCDLDOBNOQUCGYWTDZ3S77DO7ANCNFSM4XWX3GUQ.

nalimilan commented 3 years ago

These are only internal functions in Statistics, so they are only tested indirectly via cor.