-
Notifications
You must be signed in to change notification settings - Fork 21
Definition of ==
for NullableArray doesn't seem to work
#82
Comments
Good catch. Unfortunately, the only way to fix this everywhere would be to allow (See also #74) |
There's also the question of what to do with comparisons between julia> [1,2,3] == NullableArray([1,2,3])
false This comes from the fact that |
…stractArray) Fixes JuliaStats#82.
I've updated the PR with support for |
FWIW I've been using julia> X = NullableArray([1])
1-element NullableArray{Int64,1}:
1
julia> Y = NullableArray([1])
1-element NullableArray{Int64,1}:
1
julia> isequal(X, Y)
true |
I just came across the
At first glance that seems insane to me. Don't we have different things like |
julia> using NullableArrays
julia> Nullable(1) == 1
false What exactly are you referring to? |
Well, we have This is different to |
I'm just confused because the quote you provided doesn't seem to be true. Are you saying it's insane that |
I think he meant the latter. Cf #85. |
Yes, sorry for being unclear, but I did mean the latter. While I do think #85 is a good idea (and I do understand the difficulties and have read a lot about possible approaches, including the current Julia PR about higher order lifting), I feel regardless of all of that we should have I suppose the precedent might be |
How do the semantics of In any case, here's an argument for I can also see at least two counterarguments:
I'm not unsympathetic to these counterarguments; I just think they're a slippery slope. If On the whole, I think the issue of whether or not |
My guess is we're relying on some
AbstractArray
behavior, but the fact that we returnNullable{Bool}
instead ofBool
is throwing something off. To be clear, I'm not advocating for not returningNullable{Bool}
, I think we may just need to define our own==
or something where the semantics ofNullable{Bool}
vs.Bool
are currently breaking things.The text was updated successfully, but these errors were encountered: