[RFC/WIP] Few additional methods

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

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

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

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