Some compat for 0.7 #70
Conversation
| @@ -17,7 +22,7 @@ using Base.Test, Missings, Compat | |||
| @test promote_type(Union{Int, Missing}, Union{Int, Missing}) == Union{Int, Missing} | |||
| @test promote_type(Union{Float64, Missing}, Union{String, Missing}) == Any | |||
| @test promote_type(Union{Float64, Missing}, Union{Int, Missing}) == Union{Float64, Missing} | |||
| @test promote_type(Union{Void, Missing, Int}, Float64) == Any | |||
| # @test_broken promote_type(Union{Nothing, Missing, Int}, Float64) == Any | |||
There was a problem hiding this comment.
@nalimilan, are we maybe missing a promote_rule in the Base definitions? I'm seeing
julia> promote_type(Union{Nothing, Missing, Int}, Float64)
Union{Missing, Float64}on 0.7 (as of today).
There was a problem hiding this comment.
That's JuliaLang/julia#24653 (comment). See also JuliaLang/julia#25240.
There was a problem hiding this comment.
But why comment this out? Isn't it still broken?
There was a problem hiding this comment.
It's not broken on 0.6 though
There was a problem hiding this comment.
If it's
@test promote_type(Union{Nothing, Missing, Int}, Float64) == Anyit passes on 0.6, fails on 0.7.
if it's
@test_broken promote_type(Union{Nothing, Missing, Int}, Float64) == Anyit passes 0.7, but fails 0.6 (because it's NOT broken on 0.6, "unexpected pass")
so to pass on 0.6 and 0.7, I just comment it out 😄
There was a problem hiding this comment.
OK. Why not make it conditional on the version then?
| @@ -206,15 +206,19 @@ else | |||
| Base.float(A::AbstractArray{Missing}) = A | |||
| end | |||
|
|
|||
| if isdefined(Base, :adjoint) | |||
| Base.adjoint(d::Missing) = missing | |||
There was a problem hiding this comment.
This should be defined in Base as well, right?
test/runtests.jl
Outdated
| @@ -1,4 +1,9 @@ | |||
| using Base.Test, Missings, Compat | |||
| @static if VERSION < v"0.7.0-DEV.2005" | |||
There was a problem hiding this comment.
This can just be using Compat, Compat.Test, Missings
| Base.iteratorsize(::Type{<:EachReplaceMissing{T}}) where {T} = | ||
| Base.iteratorsize(T) | ||
| Base.iteratoreltype(::Type{<:EachReplaceMissing{T}}) where {T} = | ||
| Base.iteratoreltype(T) | ||
| else | ||
| Base.IteratorSize(::Type{<:EachReplaceMissing{T}}) where {T} = |
There was a problem hiding this comment.
We should add this replacement to Compat.
There was a problem hiding this comment.
I was going to, but it's weird because IteratorSize is defined in 0.6 too.
There was a problem hiding this comment.
Fredrik is adding it in Compat as Compat.IteratorSize, so we could use that if this PR is merged after his is merged and tagged.
Codecov Report
@@ Coverage Diff @@
## master #70 +/- ##
=========================================
- Coverage 94.11% 92.9% -1.22%
=========================================
Files 1 1
Lines 153 155 +2
=========================================
Hits 144 144
- Misses 9 11 +2
Continue to review full report at Codecov.
|
| @@ -286,6 +297,7 @@ Base.eltype(itr::EachReplaceMissing) = Missings.T(eltype(itr.x)) | |||
| (v isa Missing ? itr.replacement : v, s) | |||
| end | |||
|
|
|||
| @static if !isdefined(Base, :skipmissing) | |||
There was a problem hiding this comment.
Why @static here and not in other places? I'm not sure whether it's needed.
There was a problem hiding this comment.
It's not necessary at the top level (though I think you can use it to get around deprecations from the parser) but it doesn't hurt.
|
pushed changes to #72 instead. |
No description provided.