JuliaStats / Statistics.jl

The Statistics stdlib that ships with Julia.
https://juliastats.org/Statistics.jl/dev/
Other
71 stars 40 forks source link

propagate NaN value in median #164

Closed aplavin closed 3 months ago

aplavin commented 5 months ago

Propagate a NaN value from the input array, instead of creating a NaN anew.

Fixes this error, for example:

julia> using Statistics, Unitful

julia> median([NaN*u"m"])
ERROR: DimensionError: m and NaN are not dimensionally compatible.
Stacktrace:
 [1] convert(::Type{Quantity{Float64, 𝐋, Unitful.FreeUnits{(m,), 𝐋, nothing}}}, x::Float64)
   @ Unitful ~/.julia/packages/Unitful/R4J37/src/conversion.jl:106
 [2] median!(v::Vector{Quantity{Float64, 𝐋, Unitful.FreeUnits{(m,), 𝐋, nothing}}})
   @ Statistics ~/.julia/juliaup/julia-1.10.2+0.aarch64.apple.darwin14/share/julia/stdlib/v1.10/Statistics/src/Statistics.jl:815
 [3] _median
   @ ~/.julia/juliaup/julia-1.10.2+0.aarch64.apple.darwin14/share/julia/stdlib/v1.10/Statistics/src/Statistics.jl:878 [inlined]
 [4] median(v::Vector{Quantity{Float64, 𝐋, Unitful.FreeUnits{(m,), 𝐋, nothing}}})
   @ Statistics ~/.julia/juliaup/julia-1.10.2+0.aarch64.apple.darwin14/share/julia/stdlib/v1.10/Statistics/src/Statistics.jl:874
 [5] top-level scope
   @ REPL[3]:1
codecov-commenter commented 5 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 97.03%. Comparing base (68869af) to head (cc11ea9).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #164 +/- ## ========================================== + Coverage 97.01% 97.03% +0.02% ========================================== Files 2 2 Lines 435 439 +4 ========================================== + Hits 422 426 +4 Misses 13 13 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

aplavin commented 5 months ago

bump

aplavin commented 5 months ago

another bump

aplavin commented 4 months ago

yet another bump

aplavin commented 4 months ago

added tests: this PR makes NaN handling in median() consistent with others like mean() and sum()

aplavin commented 4 months ago

yet yet another bump

aplavin commented 3 months ago

yet yet yet another bump...