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

Commit

Permalink
Fix mapreduce() in a corner case
Browse files Browse the repository at this point in the history
mapreduce_seq_impl() does not exist in Base for a long time, but this went
unnoticed because since that code path is only used in very special conditions.
Another consequence of this bug is that the return type of mapreduce_pairwise_impl()
was inferred as Any. Add a test which would have triggered the failure.

Also rename a variable to fix a type instability due to using it for two
different things.
  • Loading branch information
nalimilan committed Aug 2, 2017
1 parent 2103a21 commit 2934f8e
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 3 deletions.
6 changes: 3 additions & 3 deletions src/reduce.jl
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ function mapreduce_seq_impl_skipna(f, op, T, A::DataArray, ifirst::Int, ilast::I

while i < ilast
i += 1
@inbounds na = Base.unsafe_bitgetindex(chunks, i)
na && continue
@inbounds na_el = Base.unsafe_bitgetindex(chunks, i)
na_el && continue
@inbounds d = data[i]
v = op(v, f(d))
end
Expand All @@ -36,7 +36,7 @@ function mapreduce_pairwise_impl_skipna{T}(f, op, A::DataArray{T}, bytefirst::In
ifirst = 64*(bytefirst-1)+1
ilast = min(64*bytelast, length(A))
# Fall back to Base implementation if no NAs in block
return ilast - ifirst + 1 == n_notna ? Base.mapreduce_seq_impl(f, op, A.data, ifirst, ilast) :
return ilast - ifirst + 1 == n_notna ? Base.mapreduce_impl(f, op, A.data, ifirst, ilast) :
mapreduce_seq_impl_skipna(f, op, T, A, ifirst, ilast)
end

Expand Down
6 changes: 6 additions & 0 deletions test/reduce.jl
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,12 @@ end
@test sum(da2; skipna=true) sum(dropna(da2))
end

# Check that mapreduce with skipna=true works when a full block has no NA
da = DataArray(1:2049)
da[1] = NA
@test isna(sum(da))
@test sum(da, skipna=true) === 2100224

## other reductions

_varuc(x; kw...) = var(x; corrected=false, kw...)
Expand Down

0 comments on commit 2934f8e

Please sign in to comment.