Skip to content

Improve quantile in corner cases of collection eltype #30938

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.

Sign up for GitHub

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

Merged
merged 10 commits into from
Apr 1, 2019
Merged

Improve quantile in corner cases of collection eltype #30938

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
25d7277
Allow union with Missing in quantile
bkamins Feb 1, 2019
dd08824
an improved implementation
bkamins Feb 1, 2019
64fac8d
fix typo
bkamins Feb 2, 2019
00de656
fix after a review
bkamins Feb 5, 2019
e75d6e4
add PR number
bkamins Feb 5, 2019
09b7eaf
merge methods
bkamins Feb 5, 2019
b7105fb
add more tests
bkamins Feb 6, 2019
d1e9b7e
improve type inference
bkamins Feb 6, 2019
25909a5
tests for type stability
bkamins Feb 6, 2019
94b164b
improve tests
bkamins Feb 6, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ Standard library changes

* Fixed `repr` such that it displays `DateTime` as it would be entered in Julia ([#30200]).

#### Statistics

* `quantile` now accepts in all cases collections whose `eltype` is not a subtype of `Number` ([#XXXXX]).

#### Miscellaneous

* Since environment variables on Windows are case-insensitive, `ENV` now converts its keys
Expand Down
7 changes: 5 additions & 2 deletions stdlib/Statistics/src/Statistics.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -821,8 +821,11 @@ function quantile!(q::AbstractArray, v::AbstractVector, p::AbstractArray;
return q
end

quantile!(v::AbstractVector, p::AbstractArray; sorted::Bool=false) =
quantile!(similar(p,float(eltype(v))), v, p; sorted=sorted)
function quantile!(v::AbstractVector, p::AbstractArray; sorted::Bool=false)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually can't we just use this method for tuple p? map should do the right thing in both cases.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right - I will merge them.

T = Core.Compiler.typesubtract(eltype(v), Missing)
S = T <: AbstractFloat ? T : promote_type(T, Float64)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason not to use the same approach as method below which is used when p is a tuple (which calls quantilesort! and then _quantile)? promote_type(T, Float64) is problematic here, as we claim to support types other than real.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Fixed.

quantile!(similar(p, S), v, p; sorted=sorted)
end

quantile!(v::AbstractVector, p::Real; sorted::Bool=false) =
_quantile(_quantilesort!(v, sorted, p, p), p)
Expand Down
7 changes: 6 additions & 1 deletion stdlib/Statistics/test/runtests.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,12 @@ end
@test quantile([0,1],1e-18) == 1e-18
@test quantile([1, 2, 3, 4],[]) == []
@test quantile([1, 2, 3, 4], (0.5,)) == (2.5,)
@test quantile([4, 9, 1, 5, 7, 8, 2, 3, 5, 17, 11], (0.1, 0.2, 0.4, 0.9)) == (2.0, 3.0, 5.0, 11.0)
@test quantile([4, 9, 1, 5, 7, 8, 2, 3, 5, 17, 11],
(0.1, 0.2, 0.4, 0.9)) == (2.0, 3.0, 5.0, 11.0)
@test quantile(Union{Int, Missing}[4, 9, 1, 5, 7, 8, 2, 3, 5, 17, 11],
[0.1, 0.2, 0.4, 0.9]) == [2.0, 3.0, 5.0, 11.0]
@test quantile(Any[4, 9, 1, 5, 7, 8, 2, 3, 5, 17, 11],
[0.1, 0.2, 0.4, 0.9]) == [2.0, 3.0, 5.0, 11.0]
@test quantile([1, 2, 3, 4], ()) == ()

@test_throws ArgumentError quantile([1, missing], 0.5)
Expand Down