incorrect ordering for `sample(replace=true, ordered=true)`

[stevengj]

This line in the sample! function looks incorrect. It assumes that "ordered" corresponds to sorting the samples in ascending order by value, but this is only true if the array a that you are sampling from is itself sorted, which is not required by sample!. Instead, it should be sorting by the order in which the samples appeared in a.

For example compare the following incorrect result:

julia> sample!(10:-1:1, zeros(Int,5), replace=true, ordered=true)
5-element Array{Int64,1}:
 2
 3
 5
 6
 9

to the correct ordering obtained for sampling with replace=false:

julia> sample!(10:-1:1, zeros(Int,5), replace=false, ordered=true)
5-element Array{Int64,1}:
 8
 7
 6
 5
 4

A correct algorithm would be to sample indices (with replacement), sort the indices, and then use these indices to index into the original array. In general this will require an auxiliary array, unless the type of x is such that it can store both indices and values of a.

[stevengj]

Note also that sample!(a, x, replace=true, ordered=true) fails completely if eltype(a) is not ordered:

julia> sample!(rand(ComplexF64, 10), zeros(ComplexF64,5), replace=true, ordered=true)
ERROR: MethodError: no method matching isless(::Complex{Float64}, ::Complex{Float64})

[stevengj]

As discussed here, the documentation of StatsBase.sample is rather vague on the definition of "sequential" sampling and should be improved. (But the current behavior is inconsistent and makes no sense for many types of arrays, while the proposed behavior is strictly more general as well as self-consistent.)