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

Commit

Permalink
fix mapreduce() if all values are null + tests
Browse files Browse the repository at this point in the history
* mapreduce() returns Nullable{T}(mr_empty) instead of just mr_empty
* added tests for [map]reduce() over non-empty collection that
  contain no non-null
  • Loading branch information
alyst committed Oct 7, 2016
1 parent c0f76ae commit 80f62d9
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 16 deletions.
2 changes: 1 addition & 1 deletion src/reduce.jl
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ function _mapreduce_skipnull{T}(f, op, X::NullableArray{T})
nnull = countnz(X.isnull)

(nnull == 0) && return Nullable(Base.mapreduce_impl(f, op, X.values, 1, n))
(nnull == n) && return Base.mr_empty(f, op, T)
(nnull == n) && return Nullable(Base.mr_empty(f, op, T))
(nnull == n - 1) && return Nullable(Base.r_promote(op, f(X.values[findfirst(X.isnull, false)])))
# nnull == 0 && return Base.mapreduce_impl(f, op, X, 1, n)

Expand Down
47 changes: 32 additions & 15 deletions test/reduce.jl
Original file line number Diff line number Diff line change
@@ -1,21 +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 (2, 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)
# 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
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
X = NullableArray(A)
Y = NullableArray(A, M)
B = A[find(x->!x, M)]
Expand All @@ -41,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_approx_eq v.value method(B)
@test !isnull(v)
@test isequal(method(f, Y), Nullable{Float64}())
v = method(f, Y, skipnull=true)
@test_approx_eq v.value method(f, B)
@test !isnull(v)
# test skipnull=true
if !allnull || method [sum, prod]
# reduce
v_r = method(Y, skipnull=true)
@test_approx_eq v_r.value method(B)
@test !isnull(v_r)
# mapreduce
v_mr = method(f, Y, skipnull=true)
@test_approx_eq 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

for method in (
Expand Down

0 comments on commit 80f62d9

Please sign in to comment.