sort on AbstractRange with keywords returns vector

[nick4f42]

As per #9498, adding any keywords to sort with an AbstractRange makes it call the AbstractVector method and return a vector instead of another range.

julia> versioninfo()
Julia Version 1.7.2
Commit bf53498635 (2022-02-06 15:21 UTC)
Platform Info:
  OS: Linux (x86_64-pc-linux-gnu)
  CPU: AMD Ryzen 7 3700X 8-Core Processor
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-12.0.1 (ORCJIT, znver2)

julia> xs = 1:3
1:3

julia> sort(xs)
1:3

julia> sort(xs, rev=false)
3-element Vector{Int64}:
 1
 2
 3

julia> @which sort(xs)
sort(r::AbstractUnitRange) in Base at range.jl:1304

julia> @which sort(xs, rev=false)
(::Base.var"#sort##kw")(::Any, ::typeof(sort), v::AbstractVector) in Base.Sort at sort.jl:770

[StefanKarpinski]

The rev keyword could return a range, but any other keyword make that impossible in general.

[nick4f42]

Then maybe changing the definitions of sort(r) in base/range.jl to something like this?

sort(r::AbstractUnitRange; rev::Bool=false) = rev ? reverse(r) : r
sort(r::AbstractRange; rev::Bool=false) = issorted(r) ⊻ rev ? r : reverse(r)

And similarly for sortperm. I'm not sure about sort! though.

[StefanKarpinski]

Of course that makes the sort function on ranges type unstable, which also isn't good. It seems like the current status quo is the best case already.

[nick4f42]

This would fix the type instability, but then it doesn't call the generic one when you pass other keyword arguments.

sort(r::AbstractUnitRange) = r

function sort(r::AbstractRange; rev::Bool=false)
    return ifelse(issorted(r) ⊻ rev, promote(r, reverse(r))...)
end

julia> sort(-1:1; by=abs)
ERROR: MethodError: no method matching sort(::UnitRange{Int64}; by=abs)
Closest candidates are:
  sort(::AbstractUnitRange) at REPL[1]:1 got unsupported keyword argument "by"
  sort(::AbstractRange; rev) at REPL[2]:1 got unsupported keyword argument "by"
  sort(::AbstractVector; kws...) at sort.jl:770
  ...
Stacktrace:
 [1] kwerr(::NamedTuple{(:by,), Tuple{typeof(abs)}}, ::Function, ::UnitRange{Int64})
   @ Base ./error.jl:163
 [2] top-level scope
   @ REPL[10]:1

It seems like the method in sort.jl should match? Anyways, I'll close the issue if the above won't work.

[vtjnash]

You would need to add kwargs... to the signature, and call isempty(kwargs) || return invoke(sort, Tuple{AbstractVector{T}}, r; rev, kwargs...)

[StefanKarpinski]

I dunno, it seems like when you sort a range it can't, in general, return a range so we return a vector, which is the best that can be done here. I'm very reluctant to do either introduce a type instability or return a different type based on whether there are any keyword args or not.