-
Notifications
You must be signed in to change notification settings - Fork 45
Make covariance and correlation work for iterators, skipmissing in particular. #34
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
base: master
Are you sure you want to change the base?
Changes from 2 commits
1cdf046
f3e9641
2f9c4f8
52c18ea
4620247
b86ddba
0221557
e3bc3cc
3493ed2
b940ae1
8b49745
2b28908
e42c0b0
cb3020c
36734bf
2f1c404
4279703
b9f8f96
14c5701
11bd8f5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -494,7 +494,7 @@ unscaled_covzm(x::AbstractMatrix, y::AbstractMatrix, vardim::Int) = | |||||
| (vardim == 1 ? *(transpose(x), _conj(y)) : *(x, adjoint(y))) | ||||||
|
|
||||||
| # covzm (with centered data) | ||||||
|
|
||||||
nalimilan marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| covzm(itr::Any; corrected::Bool = true) = covzm(collect(itr); corrected = corrected) | ||||||
| covzm(x::AbstractVector; corrected::Bool=true) = unscaled_covzm(x) / (length(x) - Int(corrected)) | ||||||
| function covzm(x::AbstractMatrix, vardim::Int=1; corrected::Bool=true) | ||||||
| C = unscaled_covzm(x, vardim) | ||||||
|
|
@@ -504,6 +504,7 @@ function covzm(x::AbstractMatrix, vardim::Int=1; corrected::Bool=true) | |||||
| A .= A .* b | ||||||
| return A | ||||||
| end | ||||||
| covzm(x::Any, y::Any; corrected::Bool = true) = covzm(collect(x), collect(y); corrected = corrected) | ||||||
pdeffebach marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| covzm(x::AbstractVector, y::AbstractVector; corrected::Bool=true) = | ||||||
| unscaled_covzm(x, y) / (length(x) - Int(corrected)) | ||||||
| function covzm(x::AbstractVecOrMat, y::AbstractVecOrMat, vardim::Int=1; corrected::Bool=true) | ||||||
|
|
@@ -518,16 +519,32 @@ end | |||||
| # covm (with provided mean) | ||||||
| ## Use map(t -> t - xmean, x) instead of x .- xmean to allow for Vector{Vector} | ||||||
| ## which can't be handled by broadcast | ||||||
| covm(itr::Any, itrmean; corrected::Bool=true) = | ||||||
| @show covm(collect(itr), itrmean; corrected=corrected) | ||||||
|
||||||
| @show covm(collect(itr), itrmean; corrected=corrected) | |
| covm(collect(itr), itrmean; corrected=corrected) |
pdeffebach marked this conversation as resolved.
Show resolved
Hide resolved
Outdated
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.
| `n = length(collect(itr))`. | |
| ``n`` is the number of elements. |
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.
do we want to allow 0 or more than 2 dimensional arrays here?
Outdated
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.
Better call covzm directly to avoid an additional copy:
| covm(x, mean(x); corrected=corrected) | |
| covzm(map!(t -> t - xmean, x, x); corrected=corrected) |
Same for the two-argument method.
pdeffebach marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Outdated
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.
| ``*`` denotes the complex conjugate and `n = length(collect(x)) = length(collect(y))`. If `corrected` is | |
| ``*`` denotes the complex conjugate and ``n`` the number of elements. If `corrected` is |
Outdated
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.
Same remark here and for corm as for cor about using the eltype when it's known.
Outdated
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.
Also apply the eltype check here.
Outdated
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.
Better first check whether Base.IteratorEltype(itr) isa Base.HasEltype && isconcrete(eltype(itr)), and in that case avoid calling collect.
Also remove the docstring for AbstractVector below, which is just a special case of this one.
pdeffebach marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
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.
Remove this docstring which is a special case of the previous one.
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.
Intentional?
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.
Yes. It's a hack to make sure julia knows to load this folder, it's described here for Pkg.
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.
Normally the Travis script does that automatically, so you can revert this: https://github.com/JuliaLang/Statistics.jl/blob/master/.travis.yml#L24
Though you need it to run tests locally.