JuliaLang / julia

The Julia Programming Language
https://julialang.org/
MIT License
45.61k stars 5.48k forks source link

sum(::Matrix{Union{Bool, Missing}}) fails with dims argument #32366

Open cstjean opened 5 years ago

cstjean commented 5 years ago

On 1.1 and 1.2RC1:

julia> sum([true false missing], dims=2)
ERROR: MethodError: Cannot `convert` an object of type Missing to an object of type Int64
Closest candidates are:
  convert(::Type{T<:Number}, ::T<:Number) where T<:Number at number.jl:6
  convert(::Type{T<:Number}, ::Number) where T<:Number at number.jl:7
  convert(::Type{T<:Integer}, ::Ptr) where T<:Integer at pointer.jl:23
  ...
Stacktrace:
 [1] setindex! at ./array.jl:768 [inlined]
 [2] setindex! at ./multidimensional.jl:488 [inlined]
 [3] _mapreducedim!(::typeof(identity), ::typeof(Base.add_sum), ::Array{Int64,2}, ::Array{Union{Missing, Bool},2}) at ./reducedim.jl:261
 [4] mapreducedim!(::Function, ::Function, ::Array{Int64,2}, ::Array{Union{Missing, Bool},2}) at ./reducedim.jl:274
 [5] _mapreduce_dim(::Function, ::Function, ::NamedTuple{(),Tuple{}}, ::Array{Union{Missing, Bool},2}, ::Int64) at ./reducedim.jl:317
 [6] #mapreduce#555 at ./reducedim.jl:307 [inlined]
 [7] #mapreduce at ./none:0 [inlined]
 [8] _sum at ./reducedim.jl:679 [inlined]
 [9] _sum at ./reducedim.jl:678 [inlined]
 [10] #sum#558 at ./reducedim.jl:652 [inlined]
 [11] (::getfield(Base, Symbol("#kw##sum")))(::NamedTuple{(:dims,),Tuple{Int64}}, ::typeof(sum), ::Array{Union{Missing, Bool},2}) at ./none:0
 [12] top-level scope at REPL[1]:1
nickrobinson251 commented 5 years ago

The answer should be missing?

cstjean commented 5 years ago

It should be the same as

julia> sum([1 0 missing], dims=2)
1×1 Array{Union{Missing, Int64},2}:
 missing

Without dims it already works:

julia> sum([true false missing])
missing
nalimilan commented 5 years ago

This is due to this line: https://github.com/JuliaLang/julia/blob/8d4f6d24c0fa87509fcf40788ca2c83cdfecd9af/base/reducedim.jl#L119

In https://github.com/JuliaLang/julia/pull/28089 I fixed reduction over Array{Union{Int,Missing}}, but that fix was not enough to cover types for which +/add_sum returns a different type. Actually even for Int it's not really a great strategy.

The only really correct solution would be to use the same approach as map when dims isn't specified: start with an array based on the type of the first two values, and widen it when encountering values of a new type. But that's going to be relatively tricky to write.