`==` does not compare columns of `ZonedDateTime`s correctly about datatables.jl HOT 7 OPEN

The problem is with Nullable, and fixing it in NullableArrays would require type piracy. == with NullableArray is kinda forced to be inconsistent or not to work at all because == throws an errror for Nullable in Base. The solution to this will be to move either to Union{T, Null} (in DataFrames) or to DataValue{T}.

from datatables.jl.

Yeah definitely. isqual and == are separate functions in Base for a reason.

from datatables.jl.

I'm sure I can get a PR in for this in fairly short order.

from datatables.jl.

That would be fantastic. Thanks!

from datatables.jl.

Well, I figured out why DataTables.jl has just been using isequal.

This is actually more complex to solve than I initially anticipated, owing to the fact that == checks between NullableArrays are broken.

julia> using NullableArrays

julia> a = NullableArray(1:3)
3-element NullableArrays.NullableArray{Int64,1}:
 1
 2
 3

julia> b = NullableArray(1:3)
3-element NullableArrays.NullableArray{Int64,1}:
 1
 2
 3

julia> a == b
ERROR: TypeError: non-boolean (Nullable{Bool}) used in boolean context
Stacktrace:
 [1] ==(::NullableArrays.NullableArray{Int64,1}, ::NullableArrays.NullableArray{Int64,1}) at ./abstractarray.jl:1527

This, in turn, is because == comparisons between Nullables return Nullable{Bool}, rather than Bool.

In my opinion, the best fix for this would be to provide == for NullableArrays and work with that. There was a PR to fix this in 2015, but it was never merged: JuliaStats/NullableArrays.jl#84

I think I'm going to go ahead and take a shot at a PR here, but I suspect it isn't going to be pretty.

from datatables.jl.

I think the definition for == in DataValues.jl is ok at this point. I'm also in the process of adding a DataValueArray that also fixes this, and then I'm also going to have a DataValueTable that is based on that. Essentially that will be exactly the same design as the current DataTable approach, except it will use DataValue instead of Nullable to get around the restrictions that we have due to Nullable being in base and Nullable not being special cased for the data science stack. I'm optimistic that I should be able to release soon, but on the flipside, classes start tomorrow, so who knows :)

from datatables.jl.

Makes sense to me. I've made changes to my code that's working with DataTables to work around this issue for the moment, and I won't spend time trying to make == work as expected (at least until Nullables behave themselves a little better). Thanks, folks!

from datatables.jl.