Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

float(Union{Missing, Float32}) Failing in 1.0.0, From Using quantile In a Particular Way #29693

Closed
acrosby opened this issue Oct 17, 2018 · 7 comments · Fixed by #39647
Closed
Labels
missing data Base.missing and related functionality

Comments

@acrosby
Copy link

acrosby commented Oct 17, 2018

I have been trying to run quantile for an array of Array{Union{Missing, Float32}}, but that only contains valid Float32 values. I assume this should be possible, but I get the following error that seems to be related to float conversion:

From worker 3:	MethodError: no method matching AbstractFloat(::Type{Union{Missing, Float32}})
From worker 3:	Closest candidates are:
From worker 3:	  AbstractFloat(!Matched::Bool) at float.jl:250
From worker 3:	  AbstractFloat(!Matched::Int8) at float.jl:251
From worker 3:	  AbstractFloat(!Matched::Int16) at float.jl:252
From worker 3:	  ...
From worker 3:	float(::Type) at ./float.jl:269
From worker 3:	#quantile!#46(::Bool, ::Function, ::Array{Union{Missing, Float32},1}, ::Array{Float64,1}) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.0/Statistics/src/Statistics.jl:819
From worker 3:	(::getfield(Statistics, Symbol("#kw##quantile!")))(::NamedTuple{(:sorted,),Tuple{Bool}}, ::typeof(Statistics.quantile!), ::Array{Union{Missing, Float32},1}, ::Array{Float64,1}) at ./none:0
From worker 3:	#quantile#52(::Bool, ::Function, ::Array{Union{Missing, Float32},1}, ::Array{Float64,1}) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.0/Statistics/src/Statistics.jl:913
From worker 3:	quantile(::Array{Union{Missing, Float32},1}, ::Array{Float64,1}) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.0/Statistics/src/Statistics.jl:913

Version:

julia> versioninfo()
Julia Version 1.0.0
Commit 5d4eaca0c9 (2018-08-08 20:58 UTC)
Platform Info:
  OS: Linux (x86_64-pc-linux-gnu)
  CPU: Intel(R) Xeon(R) CPU E5-1620 v2 @ 3.70GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-6.0.0 (ORCJIT, ivybridge)

I know that the documentation suggests skipmissing which must convert to a standard Array{Float}, but I assume that the above should also be possible. Would adding skipmissing in my case add additional overhead, and is this any different from explicitly converting the with convert(Array{Float32}, ...) in my case?

Thanks!

@acrosby acrosby changed the title float(Union{Missing, Float32}) failing in 1.0.0, should this be possible? float(Union{Missing, Float32}) Failing in 1.0.0, Should this be possible? Oct 17, 2018
@acrosby
Copy link
Author

acrosby commented Oct 18, 2018

The minimal example to reproduce sheds some more light on the situation:

julia> using Statistics

julia> test = Array{Union{Missing, Float32},1}(undef, 2)
2-element Array{Union{Missing, Float32},1}:
 missing
 missing

julia> test[:] .= 2.
2-element view(::Array{Union{Missing, Float32},1}, :) with eltype Union{Missing, Float32}:
 2.0f0
 2.0f0

julia> quantile(test)
ERROR: MethodError: no method matching quantile(::Array{Union{Missing, Float32},1})
Closest candidates are:
  quantile(::AbstractArray{T,1} where T, ::Any; sorted) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.0/Statistics/src/Statistics.jl:913
  quantile(::Any, ::Any; sorted) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.0/Statistics/src/Statistics.jl:911
Stacktrace:
 [1] top-level scope at none:0

julia> float(test[1])
2.0f0

julia> quantile(test, 0.1)
2.0

julia> quantile(test, 0.5)
2.0

julia> quantile(test, [0.1, 0.5])
ERROR: MethodError: no method matching AbstractFloat(::Type{Union{Missing, Float32}})
Closest candidates are:
  AbstractFloat(::Bool) at float.jl:250
  AbstractFloat(::Int8) at float.jl:251
  AbstractFloat(::Int16) at float.jl:252
  ...
Stacktrace:
 [1] float(::Type) at ./float.jl:269

@acrosby acrosby changed the title float(Union{Missing, Float32}) Failing in 1.0.0, Should this be possible? float(Union{Missing, Float32}) Failing in 1.0.0, From Using quantile In a Particular Way Oct 18, 2018
@nalimilan
Copy link
Member

We should define float(::Type{Union{T,Missing}}) where {T} = float(T). That seems to be enough to fix it. Would you make a PR?

@nalimilan nalimilan added missing data Base.missing and related functionality stdlib Julia's standard library labels Oct 18, 2018
@acrosby
Copy link
Author

acrosby commented Oct 19, 2018

Sure, I can take a crack at the process given that it may be such a simple fix. Cheers

@acrosby
Copy link
Author

acrosby commented Oct 19, 2018

Actually, I'm not sure if this will have undesired downstream impacts, either now or in the future--Since float(x) is propagating the type union, and already works out of the context of quantile.

julia> test = Array{Union{Missing, Float32},1}(undef, 2)
2-element Array{Union{Missing, Float32},1}:
 missing
 missing

julia> float(test)
2-element Array{Union{Missing, Float32},1}:
 missing
 missing

julia> float(test[1])
missing

julia> test[:] .= 2.
2-element view(::Array{Union{Missing, Float32},1}, :) with eltype Union{Missing, Float32}:
 2.0f0
 2.0f0

julia> float(test)
2-element Array{Union{Missing, Float32},1}:
 2.0f0
 2.0f0

julia> float(test[1])
2.0f0

Will have to investigate further.

@nalimilan
Copy link
Member

float(::Array{Union{T,Missing}}) where {T} works, but not float(::Type{Union{T,Missing}}) where {T}

@ararslan ararslan removed the stdlib Julia's standard library label Oct 21, 2018
@tpapp
Copy link
Contributor

tpapp commented Dec 4, 2018

@nalimilan: I just came across this issue. Are you suggesting that fixing this could be as simple as adding that method to float?

@nalimilan
Copy link
Member

Yes, probably.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
missing data Base.missing and related functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants