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

Commit f0a241a

Browse files
committed
Fix mapreduce() in a corner case
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. This bug probably comes from Base, where the code called the seq variant when the pairwise one should rather have called itself recursively: also change that for the path where there are NAs. Another consequence of this bug is that the return type of mapreduce_pairwise_impl_skipna() 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.
1 parent 2103a21 commit f0a241a

File tree

2 files changed

+10
-4
lines changed

2 files changed

+10
-4
lines changed

src/reduce.jl

+4-4
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@ function mapreduce_seq_impl_skipna(f, op, T, A::DataArray, ifirst::Int, ilast::I
2222

2323
while i < ilast
2424
i += 1
25-
@inbounds na = Base.unsafe_bitgetindex(chunks, i)
26-
na && continue
25+
@inbounds na_el = Base.unsafe_bitgetindex(chunks, i)
26+
na_el && continue
2727
@inbounds d = data[i]
2828
v = op(v, f(d))
2929
end
@@ -36,8 +36,8 @@ function mapreduce_pairwise_impl_skipna{T}(f, op, A::DataArray{T}, bytefirst::In
3636
ifirst = 64*(bytefirst-1)+1
3737
ilast = min(64*bytelast, length(A))
3838
# Fall back to Base implementation if no NAs in block
39-
return ilast - ifirst + 1 == n_notna ? Base.mapreduce_seq_impl(f, op, A.data, ifirst, ilast) :
40-
mapreduce_seq_impl_skipna(f, op, T, A, ifirst, ilast)
39+
return ilast - ifirst + 1 == n_notna ? Base.mapreduce_impl(f, op, A.data, ifirst, ilast) :
40+
mapreduce_pairwise_impl_skipna(f, op, T, A, ifirst, ilast)
4141
end
4242

4343
# Find byte in the middle of range

test/reduce.jl

+6
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,12 @@ end
7676
@test sum(da2; skipna=true) sum(dropna(da2))
7777
end
7878

79+
# Check that mapreduce with skipna=true works when a full block has no NA
80+
da = DataArray(1:2049)
81+
da[1] = NA
82+
@test isna(sum(da))
83+
@test sum(da, skipna=true) === 2100224
84+
7985
## other reductions
8086

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

0 commit comments

Comments
 (0)