NullableArrays.jl icon indicating copy to clipboard operation
NullableArrays.jl copied to clipboard

Support operations between NullableArray and Nullable/scalar

Open nalimilan opened this issue 9 years ago • 11 comments

Basic operations such as this currently don't work:

julia> NullableArray(1:2) + Nullable(1)
ERROR: MethodError: no method matching +(::NullableArrays.NullableArray{Int64,1}, ::Nullable{Int64})
Closest candidates are:
  +(::Any, ::Any, ::Any, ::Any...) at operators.jl:133
  +{S}(::Nullable{Union{}}, ::Nullable{S}) at /home/milan/.julia/NullableArrays/src/operators.jl:130
  +{S,T}(::AbstractArray{S,N}, ::AbstractArray{T,N}) at arraymath.jl:53
  ...

julia> NullableArray(1:2) .+ Nullable(1)
ERROR: MethodError: no method matching .+(::NullableArrays.NullableArray{Int64,1}, ::Nullable{Int64})
Closest candidates are:
  .+{T}(::AbstractArray{T,N}, ::Number) at arraymath.jl:78
  .+(::AbstractArray{T,N}...) at broadcast.jl:304

nalimilan avatar Aug 29 '16 16:08 nalimilan

For .+, perhaps worth waiting for 0.6 rather than implementing these in a way that would get replaced by broadcasting in the future? Non-dotted + seems like a special case we have to implement no matter what happens in Base.

johnmyleswhite avatar Sep 01 '16 15:09 johnmyleswhite

What does waiting for 0.6 have to do with it?

davidagold avatar Sep 01 '16 20:09 davidagold

Because 0.6 will offer a version of .+ that's totally derived from broadcast. I am not sure, but I think 0.5 still treats the infix dot functions in a special way.

johnmyleswhite avatar Sep 01 '16 20:09 johnmyleswhite

Ah. Yes it does.

davidagold avatar Sep 01 '16 20:09 davidagold

I don't know whether this is the right place to post this, but I am getting some strange behaviour:

julia> 3 < Nullable(4)
true
julia> 3 < Nullable()
false

the scalar boolean return value may be necessary for type stability, but I am thinking this may cause issues (I would expect the last expression to return Null).

mkborregaard avatar Jan 25 '17 12:01 mkborregaard

@mkborregaard That's really weird, as those two calls do not work at all for me. Maybe you've loaded a particular package which overloads these methods?

nalimilan avatar Jan 25 '17 12:01 nalimilan

Hah! yes, Query.jl is the culprit. Should I open an issue over there? EDIT: I'll do that now (https://github.com/davidanthoff/Query.jl/issues/79).

julia> 3 < Nullable(4)
ERROR: MethodError: no method matching isless(::Int64, ::Nullable{Int64})
Closest candidates are:
  isless(::Real, ::AbstractFloat) at operators.jl:41
  isless(::Real, ::Real) at operators.jl:75
  isless(::Integer, ::Char) at deprecated.jl:49
 in <(::Int64, ::Nullable{Int64}) at ./operators.jl:63

julia> using DataFrames

julia> 3 < Nullable(4)
ERROR: MethodError: no method matching isless(::Int64, ::Nullable{Int64})
Closest candidates are:
  isless(::Nullable{Union{}}, ::Nullable{T}) at /Users/michael/.julia/v0.5/NullableArrays/src/operators.jl:161
  isless(::Real, ::AbstractFloat) at operators.jl:41
  isless(::Real, ::Real) at operators.jl:75
  ...
 in <(::Int64, ::Nullable{Int64}) at ./operators.jl:63

julia> using Query
WARNING: Method definition require(Symbol) in module Base at loading.jl:345 overwritten in module Query at /Users/michael/.julia/v0.5/Requires/src/require.jl:12.
WARNING: Method definition ==(Base.Nullable{#T<:Any}, Base.Nullable{Union{}}) in module NullableArrays at /Users/michael/.julia/v0.5/NullableArrays/src/operators.jl:140 overwritten in module Query at /Users/michael/.julia/v0.5/Query/src/operators.jl:8.
WARNING: Method definition ==(Base.Nullable{Union{}}, Base.Nullable{#T<:Any}) in module NullableArrays at /Users/michael/.julia/v0.5/NullableArrays/src/operators.jl:139 overwritten in module Query at /Users/michael/.julia/v0.5/Query/src/operators.jl:9.
WARNING: Method definition ==(Base.Nullable{#T1<:Any}, Base.Nullable{#T2<:Any}) in module NullableArrays at /Users/michael/.julia/v0.5/NullableArrays/src/operators.jl:130 overwritten in module Query at /Users/michael/.julia/v0.5/Query/src/operators.jl:77.

julia> 3 < Nullable(4)
true

mkborregaard avatar Jan 25 '17 12:01 mkborregaard

Yes, we've discussed this previously with @davidanthoff. With Julia 0.6 supporting .< for lifting comparison, I think we should remove all these operators in favor of their element-wise/broadcasting/lifting counterparts.

nalimilan avatar Jan 25 '17 13:01 nalimilan

What type would they return? (just out of curiosity)

mkborregaard avatar Jan 25 '17 13:01 mkborregaard

It already works, and it always returns Nullable{Bool}. That's the only way to be able to return Nullable() when one of the operands is null.

nalimilan avatar Jan 25 '17 13:01 nalimilan

Just to note that this won't work in Query. I need proper predicates in Query that return Bool, not Nullable{Bool}, so Query will just opt out of the whole Nullable story if that is the direction base is taking (unless the folks that are driving these changes to Nullable can convince the base crew to allow Nullable{Bool} in filter expressions in generators, if, while, ?: clauses etc.)

davidanthoff avatar Jan 25 '17 17:01 davidanthoff