diff --git a/src/indexing.jl b/src/indexing.jl index 73891ad..36cac83 100644 --- a/src/indexing.jl +++ b/src/indexing.jl @@ -76,13 +76,21 @@ to store `v` at `I`. return v end -function unsafe_getindex_notnull(X::NullableArray, I::Int...) +# return the value of non-null X element wrapped in Nullable +@inline function unsafe_getindex_notnull(X::NullableArray, I::Int...) return Nullable(getindex(X.values, I...)) end +@inline function unsafe_getindex_notnull{T}(X::AbstractArray{Nullable{T}}, I::Int...) + return getindex(X, I...) +end -function unsafe_getvalue_notnull(X::NullableArray, I::Int...) +# return the value of non-null X element +@inline function unsafe_getvalue_notnull(X::NullableArray, I::Int...) return getindex(X.values, I...) end +@inline function unsafe_getvalue_notnull{T}(X::AbstractArray{Nullable{T}}, I::Int...) + return get(getindex(X, I...)) +end if VERSION >= v"0.5.0-dev+4697" function Base.checkindex(::Type{Bool}, inds::AbstractUnitRange, i::Nullable) @@ -109,17 +117,17 @@ else end function Base.checkbounds(::Type{Bool}, sz::Int, I::NullableVector{Bool}) - any(isnull, I) && throw(NullException()) + any(isnull, I) && throw(NullException()) length(I) == sz - end + end function Base.checkbounds{T<:Real}(::Type{Bool}, sz::Int, I::NullableArray{T}) inbounds = true - any(isnull, I) && throw(NullException()) - for i in 1:length(I) + any(isnull, I) && throw(NullException()) + for i in 1:length(I) @inbounds v = unsafe_getvalue_notnull(I, i) inbounds &= checkbounds(Bool, sz, v) - end + end return inbounds end end @@ -134,6 +142,6 @@ end This is a convenience method to set the entry of `X` at index `I` to be null """ -function nullify!(X::NullableArray, I...) +@inline function nullify!(X::NullableArray, I...) setindex!(X, Nullable{eltype(X)}(), I...) end diff --git a/src/primitives.jl b/src/primitives.jl index af2a9a9..83bbee2 100644 --- a/src/primitives.jl +++ b/src/primitives.jl @@ -1,5 +1,6 @@ -Base.isnull(X::NullableArray, I::Int...) = X.isnull[I...] -Base.values(X::NullableArray, I::Int...) = X.values[I...] +@inline Base.isnull(X::NullableArray, I::Int...) = X.isnull[I...] +@inline Base.isnull{T}(X::AbstractArray{Nullable{T}}, I::Int...) = isnull(X[I...]) # fallback method +@inline Base.values(X::NullableArray, I::Int...) = X.values[I...] """ size(X::NullableArray, [d::Real]) diff --git a/src/reduce.jl b/src/reduce.jl index 3afbc1d..b40af09 100644 --- a/src/reduce.jl +++ b/src/reduce.jl @@ -70,33 +70,24 @@ mapreduce_impl_skipnull(f, op::typeof(@functorize(+)), X::NullableArray) = # general mapreduce interface -function _mapreduce_skipnull{T}(f, op, X::NullableArray{T}, missingdata::Bool) +function _mapreduce_skipnull{T}(f, op, X::NullableArray{T}) n = length(X) - !missingdata && return Nullable(Base.mapreduce_impl(f, op, X.values, 1, n)) - nnull = countnz(X.isnull) - nnull == n && return Base.mr_empty(f, op, T) - nnull == n - 1 && return Base.r_promote(op, f(X.values[findnext(x -> x == false), X, 1])) - # nnull == 0 && return Base.mapreduce_impl(f, op, X, 1, n) + # handle specific corner cases (no nulls, all nulls, one non-null) + nnull == 0 && return Nullable(Base.mapreduce_impl(f, op, X.values, 1, n)) + nnull == n && return Nullable(Base.mr_empty(f, op, T)) + @inbounds (nnull == n - 1 && return Nullable(Base.r_promote(op, f(X.values[findfirst(X.isnull, false)])))) return mapreduce_impl_skipnull(f, op, X) end -function Base._mapreduce(f, op, X::NullableArray, missingdata) - missingdata && return Base._mapreduce(f, op, X) - Nullable(Base._mapreduce(f, op, X.values)) -end +Base._mapreduce(f, op, X::NullableArray, missingdata::Bool) = + missingdata ? Base._mapreduce(f, op, X) : Nullable(Base._mapreduce(f, op, X.values)) # to fix ambiguity warnings -function Base.mapreduce(f, op::Union{typeof(@functorize(&)), typeof(@functorize(|))}, - X::NullableArray, skipnull::Bool = false) - missingdata = any(isnull, X) - if skipnull - return _mapreduce_skipnull(f, op, X, missingdata) - else - return Base._mapreduce(f, op, X, missingdata) - end -end +Base.mapreduce(f, op::Union{typeof(@functorize(&)), typeof(@functorize(|))}, + X::NullableArray, skipnull::Bool = false) = + skipnull ? _mapreduce_skipnull(f, op, X) : Base._mapreduce(f, op, X, any(isnull, X)) if VERSION >= v"0.5.0-dev+3701" @@ -115,28 +106,16 @@ behavior is enabled, `f` will be automatically lifted over the elements of `X`. Note that, in general, mapreducing over a `NullableArray` will return a `Nullable` object regardless of whether `skipnull` is set to `true` or `false`. """ -function Base.mapreduce(f, op::Function, X::NullableArray; - skipnull::Bool = false) - missingdata = any(isnull, X) - if skipnull - return _mapreduce_skipnull(f, specialized_binary(op), - X, missingdata) - else - return Base._mapreduce(f, specialized_binary(op), X, missingdata) - end -end +Base.mapreduce(f, op::Function, X::NullableArray; skipnull::Bool = false) = + skipnull ? _mapreduce_skipnull(f, specialized_binary(op), X) : + Base._mapreduce(f, specialized_binary(op), X, any(isnull, X)) -function Base.mapreduce(f, op, X::NullableArray; skipnull::Bool = false) - missingdata = any(isnull, X) - if skipnull - return _mapreduce_skipnull(f, op, X, missingdata) - else - return Base._mapreduce(f, op, X, missingdata) - end -end +Base.mapreduce(f, op, X::NullableArray; skipnull::Bool = false) = + skipnull ? _mapreduce_skipnull(f, specialized_binary(op), X) : + Base._mapreduce(f, specialized_binary(op), X, any(isnull, X)) """ - mapreduce(f, op::Function, X::NullableArray; [skipnull::Bool=false]) + reduce(op::Function, X::NullableArray; [skipnull::Bool=false]) Reduce `X`under the operation `op`. One can set the behavior of this method to skip the null entries of `X` by setting the keyword argument `skipnull` equal diff --git a/test/reduce.jl b/test/reduce.jl index 3b61c0f..cbb49bc 100644 --- a/test/reduce.jl +++ b/test/reduce.jl @@ -1,22 +1,29 @@ module TestReduce using NullableArrays using Base.Test + import Base.mr_empty - srand(1) f(x) = 5 * x f{T<:Number}(x::Nullable{T}) = ifelse(isnull(x), Nullable{typeof(5 * x.value)}(), Nullable(5 * x.value)) + # FIXME should Base/NullableArrays handle this automatically? + Base.mr_empty(::typeof(f), op::typeof(+), T) = Base.r_promote(op, zero(T)::T) + Base.mr_empty(::typeof(f), op::typeof(*), T) = Base.r_promote(op, one(T)::T) - for N in (10, 2050) + srand(1) + for (N, allnull) in ((2, true), (2, false), (10, false), (2050, false)) A = rand(N) - M = rand(Bool, N) - i = rand(1:N) - M[i] = true - j = rand(1:N) - while j == i - j = rand(1:N) + if allnull + M = fill(true, N) + else + M = rand(Bool, N) + i = rand(1:N) + # should have at least one null and at least one non-null + M[i] = true + j = rand(1:(N-1)) + (j == i) && (j += 1) + M[j] = false end - M[j] = false X = NullableArray(A) Y = NullableArray(A, M) B = A[find(x->!x, M)] @@ -42,13 +49,22 @@ module TestReduce @test isequal(method(X), Nullable(method(A))) @test isequal(method(f, X), Nullable(method(f, A))) @test isequal(method(Y), Nullable{Float64}()) - v = method(Y, skipnull=true) - @test v.value ≈ method(B) - @test !isnull(v) @test isequal(method(f, Y), Nullable{Float64}()) - v = method(f, Y, skipnull=true) - @test v.value ≈ method(f, B) - @test !isnull(v) + # test skipnull=true + if !allnull || method ∈ [sum, prod] + # reduce + v_r = method(Y, skipnull=true) + @test v_r.value ≈ method(B) + @test !isnull(v_r) + # mapreduce + v_mr = method(f, Y, skipnull=true) + @test v_mr.value ≈ method(f, B) + @test !isnull(v_mr) + else + # reduction over empty collection not defined for these methods + @test_throws ArgumentError method(Y, skipnull=true) + @test_throws ArgumentError method(f, Y, skipnull=true) + end end @test isequal(extrema(X), (Nullable(minimum(A)), Nullable(maximum(A))))