Add pairwise

[nalimilan]

This generic method takes iterators of vectors and supports skipping missing values.
It is a more general version of pairwise in Distances.jl.
Since methods are compatible, both packages can override a common empty function defined in another API package.

See nalimilan/FreqTables.jl#54. Cc: @dkarrasch @bkamins @quinnj

[dkarrasch]

Looks pretty good, only two comments about indexing/iterating x and y and one-based indexing into res.

If I haven't missed anything, then these methods here are more generic than the generic methods in Distances.jl. I believe one could map those to the _pairwise! "kernels" here.

EDIT: Aha, I guess one difference could be that you're assuming here that you apply pairwise to iterators of (indexable) vectors, whereas pairwise in Distances.jl applies to iterators of iterators.

[nalimilan]

I've pushed commits which should address all comments. I've also made two changes:

• Add pairwise! since it is standard practice to have in-place variants
• When skipmissing=:none, allow entries to be any objects, and not just vectors. This is more consistent with Distances.jl as @dkarrasch noted.

[bkamins]

Nice. Just to go back for a second to the issue with missing. Let us confirm that we really want:

julia> x = [missing, missing, missing]
3-element Array{Missing,1}:
 missing
 missing
 missing

julia> pairwise(cor, Ref(x), Ref(x))
1×1 Array{Missing,2}:
 missing

as this is the first case when the docstring is not correct (but I think it is OK as you have no way to infer the type of "one").

The second case is:

julia> x = Union{Int, Missing}[missing, missing, missing]
3-element Array{Union{Missing, Int64},1}:
 missing
 missing
 missing

julia> pairwise(cor, Ref(x), Ref(x), skipmissing=:pairwise)
ERROR: ArgumentError: correlation only defined for non-empty vectors

(we error although we promise to return 1, but again - maybe it is OK to leave this corner case as is)

[nalimilan]

Good catch. Yes I'm not sure we can do much about it. Technically we return one(missing), so the docstring isn't totally wrong in the first case. :-p I'm not sure it's worth mentioning as it sounds difficult to explain without making the sentence very complex, but at least I've added tests so that we don't change the behavior without noticing. This made me discover that in some cases we threw a MethodError because the destination was Matrix{Union{}} instead of letting cor throw the expected ArgumentError.

I've also found a simple way to use promote_type rather than promote_typejoin to choose the element type, while still having the function inferable. I think this makes more sense as it gives a concrete eltype (Float64 in general) if you have columns of different types (which is common for tables), and it justifies using pairwise rather than just map(f, Iterators.product(x, y)).

[bkamins]

Looks good in general (but CI fails on Julia 1.0, so probably a separate code path for it is required).

[dkarrasch]

Indeed, looks good.

[nalimilan]

I've created a StatsAPI package, with a PR to define pairwisethere: JuliaStats/StatsAPI.jl#1

[pdeffebach]

Could we implement a special case for pairwise(cor, x::Vector{Number}, y::Vector{Number}) to allow scalar returns?

Or would that be a different function entirely.

[nalimilan]

Could we implement a special case for pairwise(cor, x::Vector{Number}, y::Vector{Number}) to allow scalar returns?

Or would that be a different function entirely.

That would have to be another function (more akin to skipmissings), as this PR already gives a meaning to this signature. Vectors of numbers are treated like any other collection:

julia> pairwise(tuple, [1, 2], [3, 4])
2×2 Matrix{Tuple{Int64, Int64}}:
 (1, 3)  (1, 4)
 (2, 3)  (2, 4)

I've added a commit to import pairwise and pairwise! from StatsAPI. With JuliaStats/Distances.jl#213, StatsBase and Distances will cohabit nicely without method ambiguities. I'll merge in a few days barring objections.

[bkamins]

OK. Looks good.

[wildart]

When is this expected to be merged? I'd like to use new APi in JuliaStats/MultivariateStats.jl#148.

[nalimilan]

JuliaRegistries/General#35827