Merge pull request #10496 from JuliaLang/jcb/sortperm

Relax type of sortperm! index argument to AbstractVector, add tests

base/sort.jl

@@ -355,23 +355,46 @@ defalg(v::AbstractArray) = DEFAULT_STABLE
 defalg{T<:Number}(v::AbstractArray{T}) = DEFAULT_UNSTABLE
 
 sort!(v::AbstractVector, alg::Algorithm, order::Ordering) = sort!(v,1,length(v),alg,order)
-sort!(v::AbstractVector; alg::Algorithm=defalg(v),
-    lt::Function=isless, by::Function=identity, rev::Bool=false, order::Ordering=Forward) =
+
+function sort!(v::AbstractVector;
+               alg::Algorithm=defalg(v),
+               lt::Function=isless,
+               by::Function=identity,
+               rev::Bool=false,
+               order::Ordering=Forward)
     sort!(v, alg, ord(lt,by,rev,order))
+end
 
 sort(v::AbstractVector; kws...) = sort!(copy(v); kws...)
 
 ## sortperm: the permutation to sort an array ##
 
-sortperm(v::AbstractVector; alg::Algorithm=DEFAULT_UNSTABLE,
-    lt::Function=isless, by::Function=identity, rev::Bool=false, order::Ordering=Forward) =
-    sort!([1:length(v);], alg, Perm(ord(lt,by,rev,order),v))
+function sortperm(v::AbstractVector;
+                  alg::Algorithm=DEFAULT_UNSTABLE,
+                  lt::Function=isless,
+                  by::Function=identity,
+                  rev::Bool=false,
+                  order::Ordering=Forward)
+    sort!(collect(1:length(v)), alg, Perm(ord(lt,by,rev,order),v))
+end
 
-function sortperm!{I<:Integer}(x::Vector{I}, v::AbstractVector; alg::Algorithm=DEFAULT_UNSTABLE,
-                               lt::Function=isless, by::Function=identity, rev::Bool=false, order::Ordering=Forward,
+function sortperm!{I<:Integer}(x::AbstractVector{I}, v::AbstractVector;
+                               alg::Algorithm=DEFAULT_UNSTABLE,
+                               lt::Function=isless,
+                               by::Function=identity,
+                               rev::Bool=false,
+                               order::Ordering=Forward,
                                initialized::Bool=false)
-    length(x) != length(v) && throw(ArgumentError("Index vector must be the same length as the source vector."))
-    !initialized && @inbounds for i = 1:length(v); x[i] = i; end
+    lx = length(x)
+    lv = length(v)
+    if lx != lv
+        throw(ArgumentError("index vector must be the same length as the source vector, $lx != $lv"))
+    end
+    if !initialized
+        @inbounds for i = 1:lv
+            x[i] = i
+        end
+    end
     sort!(x, alg, Perm(ord(lt,by,rev,order),v))
 end
 

test/sorting.jl

@@ -4,6 +4,12 @@
 @test sort(['a':'z';], rev=true) == ['z':-1:'a';]
 @test sortperm([2,3,1]) == [3,1,2]
 @test sortperm!([1,2,3], [2,3,1]) == [3,1,2]
+let s = sub([1,2,3,4], 1:3)
+    r = sortperm!(s, [2,3,1])
+    @test r == [3,1,2]
+    @test r === s
+end
+@test_throws ArgumentError sortperm!(sub([1,2,3,4], 1:4), [2,3,1])
 @test !issorted([2,3,1])
 @test issorted([1,2,3])
 @test reverse([2,3,1]) == [1,3,2]
@@ -80,11 +86,16 @@ a = rand(1:10000, 1000)
 for alg in [InsertionSort, MergeSort]
     b = sort(a, alg=alg)
     @test issorted(b)
+
     ix = sortperm(a, alg=alg)
     b = a[ix]
     @test issorted(b)
     @test a[ix] == b
 
+    sortperm!(sub(ix, 1:100), sub(a, 1:100), alg=alg)
+    b = a[ix][1:100]
+    @test issorted(b)
+
     sortperm!(ix, a, alg=alg)
     b = a[ix]
     @test issorted(b)
@@ -97,6 +108,10 @@ for alg in [InsertionSort, MergeSort]
     @test issorted(b, rev=true)
     @test a[ix] == b
 
+    sortperm!(sub(ix, 1:100), sub(a, 1:100), alg=alg, rev=true)
+    b = a[ix][1:100]
+    @test issorted(b, rev=true)
+
     sortperm!(ix, a, alg=alg, rev=true)
     b = a[ix]
     @test issorted(b, rev=true)
@@ -109,7 +124,12 @@ for alg in [InsertionSort, MergeSort]
     @test issorted(b, by=x->1/x)
     @test a[ix] == b
 
+    sortperm!(sub(ix, 1:100), sub(a, 1:100), by=x->1/x)
+    b = a[ix][1:100]
+    @test issorted(b, by=x->1/x)
+
     sortperm!(ix, a, alg=alg, by=x->1/x)
+    b = a[ix]
     @test issorted(b, by=x->1/x)
     @test a[ix] == b