Skip to content

Commit f0d88dd

Browse files
amilstedararslan
authored andcommitted
Update round(::Missing) to match float version (#30971)
* Update round(::Missing) to match float version `round()` was updated to accept `digits` and `base` as keyword arguments. It looks like the `Missing` version was... missed? 😈 This prevents e.g. `round(A, digits=4)` and `round(A, 4)` (old signature) from working on `Union{Missing, Float64}` arrays `A`. * Fix tests for updated round(::Missing, ...) * Test elementwise rounding on Union{T, Missing}[...] When the signature of `Base.round()` and friends (ceil,floor,trunc) was changed to accept keyword arguments for `digits` and `base` instead of positional arguments, the implementations for `Missing` were missed. Test these functions elementwise on arrays of Floats and Missing values to make sure their signatures are compatible with the versions for numbers. * Only accept Integer base in rounding * Cover missing Missing variants of round() etc. Accept `sigdigits` keyword argument in addition to `digits` and `base`. Also accept `RoundingMode` positional argument. * More variants for round(::Missing) tests Test that there are Missing versions of remaining variants of round() and friends. * Remove extraneous kwargs from trunc,floor,ceil * Clean up trunc,floor,ceil defs for Missing Continue to call `round()`, despite it just returning `missing`, so that errors happen with kwargs or types that are invalid. * Pass RoundingMode to round() in Missing versions * Check RoundingMode is being passed on for Missing * Use isequal and === * Test round() with sigdigits on missing-value arrays Also, separate tests for digits and sigdigits: Combining these arguments is not valid for floats, although it currently goes through for Missing. Finally, use a sensible base for the digits and sigdigits tests. * Fix new round() ambiguities with Missing and Rational * Reduce Rational pollution of round() methods The specification to `RoundingMode`s made it difficult to define `round()` for `Missing`. * Remove now-unneeded Missing,Rational methods * Simplify Missing rounding tests Remove duplicate tests but still test elementwise to help ensure we stay in sync with Float methods in the future. Add some comments to explain what is going on. * Remove redundant rounded_array `isequal(test_array, rounded_array) == true`, so just use `test_array`. * Remove Missing, Rational specializations My (possibly incomplete) tests suggest these are no longer needed. * Restore Rational, Missing rounding functions Still needed these to avoid ambiguities after all.
1 parent 52bafeb commit f0d88dd

File tree

3 files changed

+33
-22
lines changed

3 files changed

+33
-22
lines changed

base/missing.jl

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,16 +107,25 @@ max(::Missing, ::Any) = missing
107107
max(::Any, ::Missing) = missing
108108

109109
# Rounding and related functions
110-
for f in (:(ceil), :(floor), :(round), :(trunc))
110+
round(::Missing, ::RoundingMode=RoundNearest; sigdigits::Integer=0, digits::Integer=0, base::Integer=0) = missing
111+
round(::Type{>:Missing}, ::Missing, ::RoundingMode=RoundNearest) = missing
112+
round(::Type{T}, ::Missing, ::RoundingMode=RoundNearest) where {T} =
113+
throw(MissingException("cannot convert a missing value to type $T: use Union{$T, Missing} instead"))
114+
round(::Type{T}, x::Any, r::RoundingMode=RoundNearest) where {T>:Missing} = round(nonmissingtype(T), x, r)
115+
# to fix ambiguities
116+
round(::Type{T}, x::Rational, r::RoundingMode=RoundNearest) where {T>:Missing} = round(nonmissingtype(T), x, r)
117+
round(::Type{T}, x::Rational{Bool}, r::RoundingMode=RoundNearest) where {T>:Missing} = round(nonmissingtype(T), x, r)
118+
119+
# Handle ceil, floor, and trunc separately as they have no RoundingMode argument
120+
for f in (:(ceil), :(floor), :(trunc))
111121
@eval begin
112-
($f)(::Missing, digits::Integer=0, base::Integer=0) = missing
122+
($f)(::Missing; sigdigits::Integer=0, digits::Integer=0, base::Integer=0) = missing
113123
($f)(::Type{>:Missing}, ::Missing) = missing
114124
($f)(::Type{T}, ::Missing) where {T} =
115125
throw(MissingException("cannot convert a missing value to type $T: use Union{$T, Missing} instead"))
116126
($f)(::Type{T}, x::Any) where {T>:Missing} = $f(nonmissingtype(T), x)
117127
# to fix ambiguities
118128
($f)(::Type{T}, x::Rational) where {T>:Missing} = $f(nonmissingtype(T), x)
119-
($f)(::Type{T}, x::Rational{Bool}) where {T>:Missing} = $f(nonmissingtype(T), x)
120129
end
121130
end
122131

base/rational.jl

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -368,9 +368,9 @@ end
368368
trunc(::Type{T}, x::Rational) where {T} = convert(T,div(x.num,x.den))
369369
floor(::Type{T}, x::Rational) where {T} = convert(T,fld(x.num,x.den))
370370
ceil(::Type{T}, x::Rational) where {T} = convert(T,cld(x.num,x.den))
371+
round(::Type{T}, x::Rational, r::RoundingMode=RoundNearest) where {T} = _round_rational(T, x, r)
371372

372-
373-
function round(::Type{T}, x::Rational{Tr}, ::RoundingMode{:Nearest}) where {T,Tr}
373+
function _round_rational(::Type{T}, x::Rational{Tr}, ::RoundingMode{:Nearest}) where {T,Tr}
374374
if denominator(x) == zero(Tr) && T <: Integer
375375
throw(DivideError())
376376
elseif denominator(x) == zero(Tr)
@@ -384,9 +384,7 @@ function round(::Type{T}, x::Rational{Tr}, ::RoundingMode{:Nearest}) where {T,Tr
384384
convert(T, s)
385385
end
386386

387-
round(::Type{T}, x::Rational) where {T} = round(T, x, RoundNearest)
388-
389-
function round(::Type{T}, x::Rational{Tr}, ::RoundingMode{:NearestTiesAway}) where {T,Tr}
387+
function _round_rational(::Type{T}, x::Rational{Tr}, ::RoundingMode{:NearestTiesAway}) where {T,Tr}
390388
if denominator(x) == zero(Tr) && T <: Integer
391389
throw(DivideError())
392390
elseif denominator(x) == zero(Tr)
@@ -400,7 +398,7 @@ function round(::Type{T}, x::Rational{Tr}, ::RoundingMode{:NearestTiesAway}) whe
400398
convert(T, s)
401399
end
402400

403-
function round(::Type{T}, x::Rational{Tr}, ::RoundingMode{:NearestTiesUp}) where {T,Tr}
401+
function _round_rational(::Type{T}, x::Rational{Tr}, ::RoundingMode{:NearestTiesUp}) where {T,Tr}
404402
if denominator(x) == zero(Tr) && T <: Integer
405403
throw(DivideError())
406404
elseif denominator(x) == zero(Tr)
@@ -414,18 +412,13 @@ function round(::Type{T}, x::Rational{Tr}, ::RoundingMode{:NearestTiesUp}) where
414412
convert(T, s)
415413
end
416414

417-
function round(::Type{T}, x::Rational{Bool}) where T
415+
function round(::Type{T}, x::Rational{Bool}, ::RoundingMode=RoundNearest) where T
418416
if denominator(x) == false && (T <: Union{Integer, Bool})
419417
throw(DivideError())
420418
end
421419
convert(T, x)
422420
end
423421

424-
round(::Type{T}, x::Rational{Bool}, ::RoundingMode{:Nearest}) where {T} = round(T, x)
425-
round(::Type{T}, x::Rational{Bool}, ::RoundingMode{:NearestTiesAway}) where {T} = round(T, x)
426-
round(::Type{T}, x::Rational{Bool}, ::RoundingMode{:NearestTiesUp}) where {T} = round(T, x)
427-
round(::Type{T}, x::Rational{Bool}, ::RoundingMode) where {T} = round(T, x)
428-
429422
trunc(x::Rational{T}) where {T} = Rational(trunc(T,x))
430423
floor(x::Rational{T}) where {T} = Rational(floor(T,x))
431424
ceil(x::Rational{T}) where {T} = Rational(ceil(T,x))

test/missing.jl

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -188,16 +188,25 @@ Base.one(::Type{Unit}) = 1
188188
end
189189

190190
@testset "rounding functions" begin
191-
rounding_functions = [ceil, floor, round, trunc]
192-
193191
# All rounding functions return missing when evaluating missing as first argument
192+
193+
# Check that the RoundingMode argument is passed on correctly
194+
@test round(Union{Int, Missing}, 0.9) === round(Int, 0.9)
195+
@test round(Union{Int, Missing}, 0.9, RoundToZero) === round(Int, 0.9, RoundToZero)
196+
197+
# Test elementwise on mixed arrays to ensure signature of Missing methods matches that of Float methods
198+
test_array = [1.0, missing]
199+
200+
@test isequal(round.(test_array, RoundNearest), test_array)
201+
@test isequal(round.(Union{Int, Missing}, test_array, RoundNearest), test_array)
202+
203+
rounding_functions = [ceil, floor, round, trunc]
194204
for f in rounding_functions
195-
@test ismissing(f(missing))
196-
@test ismissing(f(missing, 1))
197-
@test ismissing(f(missing, 1, 1))
198-
@test ismissing(f(Union{Int, Missing}, missing))
199-
@test f(Union{Int, Missing}, 1.0) === 1
200205
@test_throws MissingException f(Int, missing)
206+
@test isequal(f.(test_array), test_array)
207+
@test isequal(f.(test_array, digits=0, base=10), test_array)
208+
@test isequal(f.(test_array, sigdigits=1, base=10), test_array)
209+
@test isequal(f.(Union{Int, Missing}, test_array), test_array)
201210
end
202211
end
203212

0 commit comments

Comments
 (0)