-
Notifications
You must be signed in to change notification settings - Fork 194
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow computing the correlation matrix from the covariance matrix only #661
Conversation
""" | ||
cov2cor(C) | ||
|
||
Compute the correlation matrix from the covariance matrix `C`. Use `StatsBase.cov2cor!` for an in-place version. | ||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd merge this with the docstring for the 2-argument version (with cov2cor(C[, s])
as a signature).
Can you add tests? |
Did you remove the tests on purpose? |
Well, which tests? I didn’t add any tests to Statistics.jl yet. There where two tests that I added to StatsBase.jl in my PR, that failed for what are now obvious reasons.
… On 23 Feb 2021, at 08:50, Milan Bouchet-Valat ***@***.***> wrote:
Did you remove the tests on purpose?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub <#661 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ALJ5RCGRNYNRAPBFCV36FM3TANT3XANCNFSM4XWXW22A>.
|
Hi there, Looks like the method hasn't been implemented. In cov2cor!(C) = cov2cor!(C, [sqrt(C[i,i]) for i = 1:size(C, 1)])
cov2cor(C) = cov2cor!(copy(C)) In # Around line 126
@test cov2cor(cov1) ≈ cor1
@test cov2cor(cov2) ≈ cor2
# Around line 202
@test cov2cor(cov1) ≈ cor1
@test cov2cor(cov2) ≈ cor2
# Around line 209
StatsBase.cov2cor!(tmp_cov1)
@test tmp_cov1 ≈ cor1
# Around line 214
StatsBase.cov2cor!(tmp_cov2)
@test tmp_cov2 ≈ cor2 |
Please make a new PR, that's simpler. |
Superseded by #816 |
Following issue #652 in StatsBase.jl