Skip to content
This repository has been archived by the owner on May 4, 2019. It is now read-only.

Support operations between NullableArray and Nullable/scalar #143

Open
nalimilan opened this issue Aug 29, 2016 · 11 comments
Open

Support operations between NullableArray and Nullable/scalar #143

nalimilan opened this issue Aug 29, 2016 · 11 comments

Comments

@nalimilan
Copy link
Member

nalimilan commented Aug 29, 2016

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 added a commit to JuliaData/DataFrames.jl that referenced this issue Aug 29, 2016
Again a broader issue which doesn't particularly affect DataFrames. Cf.
JuliaStats/NullableArrays.jl#143
nalimilan added a commit to JuliaData/DataFrames.jl that referenced this issue Aug 31, 2016
Again a broader issue which doesn't particularly affect DataFrames. Cf.
JuliaStats/NullableArrays.jl#143
nalimilan added a commit to JuliaData/DataFrames.jl that referenced this issue Aug 31, 2016
Again a broader issue which doesn't particularly affect DataFrames. Cf.
JuliaStats/NullableArrays.jl#143
@johnmyleswhite
Copy link
Member

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.

nalimilan added a commit to JuliaData/DataFrames.jl that referenced this issue Sep 1, 2016
Again a broader issue which doesn't particularly affect DataFrames. Cf.
JuliaStats/NullableArrays.jl#143
@davidagold
Copy link
Contributor

What does waiting for 0.6 have to do with it?

@johnmyleswhite
Copy link
Member

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.

nalimilan added a commit to JuliaData/DataFrames.jl that referenced this issue Sep 1, 2016
Again a broader issue which doesn't particularly affect DataFrames. Cf.
JuliaStats/NullableArrays.jl#143
@davidagold
Copy link
Contributor

Ah. Yes it does.

nalimilan added a commit to JuliaData/DataFrames.jl that referenced this issue Sep 1, 2016
Again a broader issue which doesn't particularly affect DataFrames. Cf.
JuliaStats/NullableArrays.jl#143
nalimilan added a commit to JuliaData/DataFrames.jl that referenced this issue Sep 22, 2016
Again a broader issue which doesn't particularly affect DataFrames. Cf.
JuliaStats/NullableArrays.jl#143
maximerischard pushed a commit to maximerischard/DataFrames.jl that referenced this issue Sep 28, 2016
Again a broader issue which doesn't particularly affect DataFrames. Cf.
JuliaStats/NullableArrays.jl#143
@mkborregaard
Copy link

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).

@nalimilan
Copy link
Member Author

@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?

@mkborregaard
Copy link

mkborregaard commented Jan 25, 2017

Hah! yes, Query.jl is the culprit. Should I open an issue over there? EDIT: I'll do that now (queryverse/Query.jl#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

@nalimilan
Copy link
Member Author

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.

@mkborregaard
Copy link

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

@nalimilan
Copy link
Member Author

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.

@davidanthoff
Copy link

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.)

nalimilan added a commit to JuliaData/DataFrames.jl that referenced this issue Jul 8, 2017
Again a broader issue which doesn't particularly affect DataFrames. Cf.
JuliaStats/NullableArrays.jl#143
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants