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

Commit f805f6e

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. 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 f805f6e

File tree

2 files changed

+9
-3
lines changed

2 files changed

+9
-3
lines changed

src/reduce.jl

+3-3
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,7 +36,7 @@ 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) :
39+
return ilast - ifirst + 1 == n_notna ? Base.mapreduce_impl(f, op, A.data, ifirst, ilast) :
4040
mapreduce_seq_impl_skipna(f, op, T, A, ifirst, ilast)
4141
end
4242

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)