Sort coverage.

[Ismael-VC]

This tests will never run because we can't construct ranges with step of 0. It's the only thing that stops sort coverage from reaching 100%.

See:

• Ref: julia-dev
• sort coverage

[kmsquire]

As pointed out in fda850b, this is a detail of the Range implementation.

It may be that this change in the future (Range and related code has been reimplemented multiple times), or that someone implements a different range type (maybe outside of Base) which allows this.

What this means is that, while this code might not be used by anything in Base, it's probably worth leaving it in there to prevent future problems.

And it's really okay that code coverage doesn't reach 100%! Part of the purpose of code coverage is to make sure we're testing enough code to feel confident that we don't have (many) bugs. And while increasing coverage has uncovered a number of bugs, in this case, we can probably verify the last few lines of code which are not covered visually. (These lines look fine, BTW.)

Incidentally, if you wanted to add a short comment explaining why these lines are in the code even though none of the Range implementations in base allow steps of size 0, that would probably be welcome!

[nalimilan]

OTOH code that is never tested is unlikely to work at all. Could we create a dummy range type which with a zero step just for the tests?

[kmsquire]

@nalimilan, sure, that would work.

[Ismael-VC]

Honest curiosity, what use does a zero step range has? This doesn't seem to be supported by most of the programming languages that I know so far.

Python:

In [1]: range(1, 10 , 0)
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-1-ebfc00fa11ce> in <module>()
----> 1 range(1, 10 , 0)

ValueError: range() step argument must not be zero

In [2]: import numpy

In [3]: numpy.arange(1, 10 , 0)
---------------------------------------------------------------------------
ZeroDivisionError                         Traceback (most recent call last)
<ipython-input-3-463b9d872f7f> in <module>()
----> 1 numpy.arange(1, 10 , 0)

ZeroDivisionError: division by zero

Ruby:

>> (1..10).step(0).to_a
ArgumentError: step can't be 0
    from (irb):4:in `step'
    from (irb):4:in `each'
    from (irb):4:in `to_a'
    from (irb):4

Clojure:

user=> (range 1 10 0)

OutOfMemoryError Java heap space  java.util.Arrays.copyOf (Arrays.java:2882)

AFAIK the only one is Matlab/Octave:

octave:1> 1:0:10
ans = [](1x0)

Ok, an empty array sounds reasonable, but why would someone go and make a custom range type that supports this in a language that doesn't?, or why would julia may implement it in the future or not? what's the use case?

[mbauman]

A zero step may be needed to fix #10391.

[mbauman]

Actually, can you cover these lines with a (edit: sneakily constructed, cf. #10391 (comment)) zero-step FloatRange?

[Ismael-VC]

@mbauman Julia can't construct ranges with zero-step currently. I've spent the last couple of hours thinking how to implement a DummyRange as @nalimilan suggested, but I'm not sure how to do so yet. :P

[mbauman]

See my edit, which I added just as you were commenting: You can get a zero-step float range by subtracting two float ranges with the same step:

julia> r = (2.:6.) - (1.:5.)
1.0:0.0:1.0

julia> length(r)
5

julia> collect(r)
5-element Array{Float64,1}:
 1.0
 1.0
 1.0
 1.0
 1.0

You can't construct one with the colon syntax since it doesn't know how long it should be.

[mbauman]

This doesn't work for integer ranges since their structure is much simpler and highly optimized so for loops can be fast. They don't independently store the length, which is what is needed here (since the range is no longer fully defined by its start/stop and step — it needs a length, too).

[Ismael-VC]

Oh I see, actually we can use the FloatRanges constructor:

julia> r = (2.:6.) - (1.:5.)
1.0:0.0:1.0

julia> collect(r)
5-element Array{Float64,1}:
 1.0
 1.0
 1.0
 1.0
 1.0

julia> collect(FloatRange(1.0, 0.0, 5.0, 1.0))
5-element Array{Float64,1}:
 1.0
 1.0
 1.0
 1.0
 1.0

help?> FloatRange
search: FloatRange

DataType   : FloatRange{T<:FloatingPoint}
  supertype: Range{T<:FloatingPoint}
  fields   : [:start,:step,:len,:divisor]

[Ismael-VC]

Oh I see, actually we can use the FloatRanges constructor:

julia> using Base.Order

julia> r = FloatRange(1.0, 0.0, 5.0, 1.0)
1.0:0.0:1.0

julia> searchsortedfirst(r, 1.0, Forward)
1

julia> searchsortedlast(r, 1.0, Forward)
5

But that would only cover:

• searchsortedfirst{T<:Real}(a::Range{T}, x::Real, o::DirectOrdering)
• searchsortedlast{T<:Real}(a::Range{T}, x::Real, o::DirectOrdering)

And this would leave uncovered:

• searchsortedfirst{T<:Integer}(a::Range{T}, x::Real, o::DirectOrdering)
• searchsortedlast{T<:Integer}(a::Range{T}, x::Real, o::DirectOrdering)
• searchsortedfirst{T<:Integer}(a::Range{T}, x::Unsigned, o::DirectOrdering)
• searchsortedlast{T<:Integer}(a::Range{T}, x::Unsigned, o::DirectOrdering)

Meanwhile I'm going to update de PR with these changes to the methods parametrized by {T<:Real} then.

[Ismael-VC]

Would the if step(r::Range) == 0 test may still be useful for {T<:Integer}?

[mbauman]

I think it'd be best to leave them in. There's a possibility that a custom Range type would allow for zero steps, and eventually maybe we'll be able to support zero step integer ranges in base.

To test this, you could create a ConstantRange:

julia> immutable ConstantRange{T} <: Range{T}
           val::T
           len::Int
       end

julia> Base.length(r::ConstantRange) = r.len
length (generic function with 55 methods)

julia> Base.getindex(r::ConstantRange, i::Int) = (1 <= i <= r.len || throw(BoundsError(r,i)); r.val)
getindex (generic function with 117 methods)

julia> Base.step(r::ConstantRange) = 0
step (generic function with 6 methods)

julia> r = ConstantRange(2,3)
2:0:2

julia> collect(r)
3-element Array{Int64,1}:
 2
 2
 2

[Ismael-VC]

@mbauman thank you very much!

I've updated test/sorting.jl with your examples and reverted base/sort.jl back to it's original state.

[mbauman]

👍 Assuming those tests pass this looks good to me. Hurray for 100%!

[Ismael-VC]

Damn! It's going to fail, forgot to use Base.Order.Forward instead of just Forward. Just a sec....

[Ismael-VC]

Ok that should work! Hurray! 😂

[Ismael-VC]

What a mess! 😟 I thought queued tests were automatically canceled when pushing to the same PR, Could some one please cancel my other queued jobs? Thanks in advance!

[ivarne]

This section would benefit from a comment. (I know lots of others would too, but it is easier to complain in a not-yet-merged PR)

Eg:

# exercise the codepath in searchsorted* methods for ranges that check for zero step range

[Ismael-VC]

Thanks!, I agree, it's done.

[tkelman]

Strange assertion failure on osx travis, probably unrelated though

$ cp /tmp/julia/lib/julia/sys.ji local.ji && /tmp/julia/bin/julia -J local.ji -e 'true' && /tmp/julia/bin/julia-debug -J local.ji -e 'true' && rm local.ji
Assertion failed: (bp), function jl_deserialize_value, file dump.c, line 1019.
/Users/travis/build.sh: line 41:  9775 Abort trap: 6           /tmp/julia/bin/julia -J local.ji -e 'true'
The command "cp /tmp/julia/lib/julia/sys.ji local.ji && /tmp/julia/bin/julia -J local.ji -e 'true' && /tmp/julia/bin/julia-debug -J local.ji -e 'true' && rm local.ji" exited with 134.
0.00s$ /tmp/julia/bin/julia -e 'versioninfo()'
Assertion failed: (bp), function jl_deserialize_value, file dump.c, line 1019.
/Users/travis/build.sh: line 41:  9786 Abort trap: 6           /tmp/julia/bin/julia -e 'versioninfo()'
The command "/tmp/julia/bin/julia -e 'versioninfo()'" exited with 134.
0.00s$ export JULIA_CPU_CORES=4 && cd /tmp/julia/share/julia/test && /tmp/julia/bin/julia --check-bounds=yes runtests.jl all && /tmp/julia/bin/julia --check-bounds=yes runtests.jl pkg
Assertion failed: (bp), function jl_deserialize_value, file dump.c, line 1019.
/Users/travis/build.sh: line 41:  9796 Abort trap: 6           /tmp/julia/bin/julia --check-bounds=yes runtests.jl all
The command "export JULIA_CPU_CORES=4 && cd /tmp/julia/share/julia/test && /tmp/julia/bin/julia --check-bounds=yes runtests.jl all && /tmp/julia/bin/julia --check-bounds=yes runtests.jl pkg" exited with 134.

[StefanKarpinski]

A clever use case for zero-step integer ranges is broadcasting array dimensions by slicing, i.e. this:

v = [1:10;]
v[:,(1:10)-(1:10)] # 10x10

We don't yet have array slicing as views, but this would efficiently allow broadcasting without data copy.

[mbauman]

Thanks, @Ismael-VC!

That's a really interesting application, @StefanKarpinski. It reminds me of the Ones array type I conjured up to reduce allocation (and avoid arithmetic!) in this StackOverflow question: http://stackoverflow.com/a/30968709/176071. I wonder if there are other uses for this sort of constant array. It almost feels like a corollary to I.

(AV failure is #11807)

[Ismael-VC]

@mbauman you're welcome! This is exciting, finally I start to contribute to such a great project! And not only a great (the best!!!) project, but also a great community! ✨

Thank you all for your support and hard work! 👏

[tkelman]

@Ismael-VC your enthusiasm is awesome to see 😄

[Ismael-VC]

@StefanKarpinski

A clever use case for zero-step integer ranges is broadcasting array dimensions by slicing, i.e. this:

v = [1:10;]
v[:,(1:10)-(1:10)] # 10x10

We don't yet have array slicing as views, but this would efficiently allow broadcasting without data copy.

This works!

julia> v[:, FloatRange(1.0, 0.0, 10.0, 1.0)]
10x10 Array{Int64,2}:
  1   1   1   1   1   1   1   1   1   1
  2   2   2   2   2   2   2   2   2   2
  3   3   3   3   3   3   3   3   3   3
  4   4   4   4   4   4   4   4   4   4
  5   5   5   5   5   5   5   5   5   5
  6   6   6   6   6   6   6   6   6   6
  7   7   7   7   7   7   7   7   7   7
  8   8   8   8   8   8   8   8   8   8
  9   9   9   9   9   9   9   9   9   9
 10  10  10  10  10  10  10  10  10  10

[StefanKarpinski]

Yup. But for this use case, you'd ideally want to be able to use an integer range and produce a slice.