[BREAKING] add matchmissing kwarg to joins

[bkamins]

Fixes #2499.

This is the core implementation and update of docstrings.

I will add tests later, as it first requires #2503 to be merged and rebased against.

[nalimilan]

Looks good. Though I think we should do something about NaN (which dplyr also handles via the equivalent of matchmissing). Maybe just throw an error if there are NaNs for now and see whether people complain.

[bkamins]

Maybe just throw an error if there are NaNs for now and see whether people complain.

OK. Doing joins on floats is in general not advisable though:

julia> outerjoin(DataFrame(x=0.0,a=1), DataFrame(x=-0.0,b=2), on=:x)
2×3 DataFrame
│ Row │ x       │ a       │ b       │
│     │ Float64 │ Int64?  │ Int64?  │
├─────┼─────────┼─────────┼─────────┤
│ 1   │ 0.0     │ 1       │ missing │
│ 2   │ -0.0    │ missing │ 2       │

(which means in particular that I am OK to error on NaN)

[nalimilan]

Yeah -0.0 is also a tricky case. We could also throw an error if we find -0.0.

[bkamins]

@StefanKarpinski - do you have an opinion how NaN and -0.0 should be handled in joins when they are present in columns you join on? (currently we accept them and use isequal to match - just like Dicts)

As you can see @nalimilan considers if we should not just error on them. In general I understand his point, but I am not 100% sure what would be best. Given your experiences with designing isequal in Base what do you think would be best?

[kescobo]

Doing joins on floats is in general not advisable though

💯 If people are doing joins on floats, something has already gone seriously wrong. But kudos for trying to get it right. How are other == but not === things handled? Eg.

a = [1,2]
b = [1,2]
c = [1,3]

df1 = DataFrame(x = [a,b,c], y = rand(3))
df2 = DataFrame(x = [a], z = [rand()])

julia> leftjoin(df1,df2, on=:x)
3×3 DataFrame
│ Row │ x      │ y        │ z        │
│     │ Array… │ Float64  │ Float64? │
├─────┼────────┼──────────┼──────────┤
│ 1   │ [1, 2] │ 0.213345 │ 0.614537 │
│ 2   │ [1, 2] │ 0.70363  │ 0.614537 │
│ 3   │ [1, 3] │ 0.181217 │ missing  │

So for consistency, seems like -0.0 and 0.0 should be treated the same, no?

[kescobo]

Oh, but the original question was about NaN - that one is weird since it's === but not ==... That I have no idea about

[tbeason]

I think NaN could more or less follow whatever decision was made about missing, right? Why treat them differently with regards to matching?

I am indifferent about 0.0 and -0.0 being special cased.

[bkamins]

I have thought about it and I think I would leave floating point columns as they are now. One normally should not join on such columns anyway, as it is error prone in general. Instead I propose to add a comment in the docstrings warning about doing joins on floating point columns.

EDIT: but if you strongly feel we should error on NaN always then I can leave it as is. Then I would only add a comment that -0.0 and 0.0 are considered not equal. The case when joining on NaN or -0.0 might make sense is when you want to add a column with a descriptive mapping of on columns (like e.g. NaN is mapped to "error" value in the column added via join).

[nalimilan]

Since we can't start throwing an error without breaking, I'd rather throw an error with NaNs (current state of PR), and see later whether we should add an argument to disable it (or even disable it altogether). It would be safer to do that also for -0.0.

[bkamins]

OK - I will disallow -0.0 also then and throw an informative error message.

[bkamins]

I have updated the implementation and added appropriate tests. I think that the solution I propose (to use CategoricalVector as a wrapper) is clean enough (ie. if someone really wants to make such a join there is a way to do it).

The PR should be ready for a review.

[nalimilan]

0.0 works even without matchmissing=:equal, right?

[bkamins]

right, but I wanted to avoid adding the second set of the same tests. I think it is OK, as :equal has no influence on 0.0 handling

[bkamins]

Thank you!