Skip to content
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

Improve range: refactor, support start as an optional kwarg, clearer docs and error messages #38041

Merged
merged 20 commits into from
Jan 12, 2021
Merged

Improve range: refactor, support start as an optional kwarg, clearer docs and error messages #38041

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
62e629f
add range(;start, step, stop, length)
jw3126 Oct 15, 2020
208eaf0
allow a few more range argument patterns
jw3126 Oct 15, 2020
8d5c3ba
fix
jw3126 Oct 15, 2020
a5ef905
Update base/range.jl
jw3126 Oct 20, 2020
50734fd
remove four argument range
jw3126 Oct 24, 2020
3796650
force a keyword argument in range(start, stop)
jw3126 Oct 27, 2020
cc25cf3
Update base/range.jl
jw3126 Nov 6, 2020
8c327e9
Update base/range.jl
jw3126 Nov 12, 2020
519ef36
Merge branch 'master' into range
jw3126 Dec 18, 2020
87e8faf
Merge branch 'range' of github.com:jw3126/julia into range
jw3126 Dec 18, 2020
7712b4c
Update test/ranges.jl
jw3126 Jan 6, 2021
083a4f6
Update NEWS.md
mbauman Jan 7, 2021
1097661
Update base/range.jl
jw3126 Jan 7, 2021
ba32fef
Update base/range.jl
jw3126 Jan 7, 2021
6a0e25e
fix spelling
jw3126 Jan 7, 2021
7500cdd
Merge branch 'master' of https://github.com/JuliaLang/julia into range
jw3126 Jan 7, 2021
0c5fc36
fix documented julia version in range
jw3126 Jan 7, 2021
307d59b
Move the NEWS to the right place
mbauman Jan 7, 2021
65898ed
fix
jw3126 Jan 8, 2021
01f6ccc
add minor comment to range unit test
jw3126 Jan 11, 2021
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
184 changes: 132 additions & 52 deletions base/range.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,25 +47,16 @@ function _colon(start::T, step, stop::T) where T
end

"""
range(start[, stop]; length, stop, step=1)
range(start, stop; length, step)
range(start; length, stop, step)
range(;start, length, stop, step)

Given a starting value, construct a range either by length or from `start` to `stop`,
optionally with a given step (defaults to 1, a [`UnitRange`](@ref)).
One of `length` or `stop` is required. If `length`, `stop`, and `step` are all specified, they must agree.

If `length` and `stop` are provided and `step` is not, the step size will be computed
automatically such that there are `length` linearly spaced elements in the range.

If `step` and `stop` are provided and `length` is not, the overall range length will be computed
automatically such that the elements are `step` spaced.

Special care is taken to ensure intermediate values are computed rationally.
To avoid this induced overhead, see the [`LinRange`](@ref) constructor.

`stop` may be specified as either a positional or keyword argument.

!!! compat "Julia 1.1"
`stop` as a positional argument requires at least Julia 1.1.
Construct a `range` from the arguments.
jw3126 marked this conversation as resolved.
Show resolved Hide resolved
Mathematically a range is uniquely determined by any three of `start`, `step`, `stop` and `length`.
Valid invocations of range are:
* Call `range` with any three of `start`, `step`, `stop`, `length`.
* Call `range` with two of `start`, `stop`, `length`. In this case `step` will be assumed
to be one. If all arguments are integers, a [`UnitRange`](@ref) will be returned.
mbauman marked this conversation as resolved.
Show resolved Hide resolved

# Examples
```jldoctest
Expand All @@ -86,51 +77,140 @@ julia> range(1, 10, length=101)

julia> range(1, 100, step=5)
1:5:96

julia> range(stop=10, length=5)
6:10

julia> range(stop=10, step=1, length=5)
6:1:10

julia> range(start=1, step=1, stop=10)
1:1:10
```
If `length` is not specified and `stop - start` is not an integer multiple of `step`, a range that ends before `stop` will be produced.
```jldoctest
julia> range(1, 3.5, step=2)
1.0:2.0:3.0
```

Special care is taken to ensure intermediate values are computed rationally.
To avoid this induced overhead, see the [`LinRange`](@ref) constructor.

`start` may be specified as either a positional or keyword argument.
`stop` may be specified as either a positional or keyword argument. If it is specified as a positional argument,
one of `step` or `length` must also be provided.
jw3126 marked this conversation as resolved.
Show resolved Hide resolved

!!! compat "Julia 1.1"
`stop` as a positional argument requires at least Julia 1.1.

!!! compat "Julia 1.6"
`start` as a keyword argument requires at least Julia 1.6.
jw3126 marked this conversation as resolved.
Show resolved Hide resolved
"""
range(start; length::Union{Integer,Nothing}=nothing, stop=nothing, step=nothing) =
function range end

range(start; stop=nothing, length::Union{Integer,Nothing}=nothing, step=nothing) =
_range(start, step, stop, length)

range(start, stop; length::Union{Integer,Nothing}=nothing, step=nothing) =
_range2(start, step, stop, length)
function range(start, stop; length::Union{Integer,Nothing}=nothing, step=nothing)
# For code clarity, the user must pass step or length
# See https://github.com/JuliaLang/julia/pull/28708#issuecomment-420034562
if step === length === nothing
msg = """
Neither `step` nor `length` was provided. To fix this do one of the following:
* Pass one of them
* Use `$(start):$(stop)`
* Use `range($start, stop=$stop)`
"""
throw(ArgumentError(msg))
end
_range(start, step, stop, length)
end

_range2(start, ::Nothing, stop, ::Nothing) =
throw(ArgumentError("At least one of `length` or `step` must be specified"))
range(;start=nothing, stop=nothing, length::Union{Integer, Nothing}=nothing, step=nothing) =
_range(start, step, stop, length)

_range2(start, step, stop, length) = _range(start, step, stop, length)
_range(start::Nothing, step::Nothing, stop::Nothing, len::Nothing) = range_error(start, step, stop, len)
_range(start::Nothing, step::Nothing, stop::Nothing, len::Any ) = range_error(start, step, stop, len)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

1 as a default start value sounds like a good idea. Should be a separate PR however. I really don t want to delay this one further.

_range(start::Nothing, step::Nothing, stop::Any , len::Nothing) = range_error(start, step, stop, len)
_range(start::Nothing, step::Nothing, stop::Any , len::Any ) = range_stop_length(stop, len)
_range(start::Nothing, step::Any , stop::Nothing, len::Nothing) = range_error(start, step, stop, len)
_range(start::Nothing, step::Any , stop::Nothing, len::Any ) = range_error(start, step, stop, len)
_range(start::Nothing, step::Any , stop::Any , len::Nothing) = range_error(start, step, stop, len)
_range(start::Nothing, step::Any , stop::Any , len::Any ) = range_step_stop_length(step, stop, len)
_range(start::Any , step::Nothing, stop::Nothing, len::Nothing) = range_error(start, step, stop, len)
_range(start::Any , step::Nothing, stop::Nothing, len::Any ) = range_start_length(start, len)
_range(start::Any , step::Nothing, stop::Any , len::Nothing) = range_start_stop(start, stop)
_range(start::Any , step::Nothing, stop::Any , len::Any ) = range_start_stop_length(start, stop, len)
_range(start::Any , step::Any , stop::Nothing, len::Nothing) = range_error(start, step, stop, len)
_range(start::Any , step::Any , stop::Nothing, len::Any ) = range_start_step_length(start, step, len)
_range(start::Any , step::Any , stop::Any , len::Nothing) = range_start_step_stop(start, step, stop)
_range(start::Any , step::Any , stop::Any , len::Any ) = range_error(start, step, stop, len)
Comment on lines +131 to +146
Copy link
Member

@simeonschaub simeonschaub Jan 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Could we use @eval in a for loop here? This just seems a bit hard to maintain.

Copy link
Member

Choose a reason for hiding this comment

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

I think it looks cool 😛

Copy link
Contributor

@mkitti mkitti Jan 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

It could use some comments. I think @eval would make it more complicated. There are four options and 4^2 == 16 lines. 16 lines with a non-trivial mapping seems pretty manageable to me. We need to make it easier to read, not less.

How about something like this:

# range( )
_range(start::Nothing, step::Nothing, stop::Nothing, len::Nothing) = range_error(start, step, stop, len)
# range(; length )
_range(start::Nothing, step::Nothing, stop::Nothing, len::Any    ) = range_error(start, step, stop, len)
# range(; stop )
_range(start::Nothing, step::Nothing, stop::Any    , len::Nothing) = range_error(start, step, stop, len)
# range(; stop, length )
_range(start::Nothing, step::Nothing, stop::Any    , len::Any    ) = range_stop_length(stop, len)

Copy link
Member

Choose a reason for hiding this comment

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

It looks very self-documenting to my eyes with a clear pattern.

Copy link
Contributor

Choose a reason for hiding this comment

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

It is navigable to me, but I need to parse it to find the correct line. I'm happy with how it is though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I put some thought into how to write down this dispatch mechanic. I also considered using @eval but decided the above table is the most readable and easy to maintain.


range_stop_length(stop, length) = (stop-length+1):stop

range_step_stop_length(step, stop, length) = reverse(range_start_step_length(stop, -step, length))

range_start_length(a::Real, len::Integer) = UnitRange{typeof(a)}(a, oftype(a, a+len-1))
range_start_length(a::AbstractFloat, len::Integer) = range_start_step_length(a, oftype(a, 1), len)
range_start_length(a, len::Integer) = range_start_step_length(a, oftype(a-a, 1), len)

range_start_stop(start, stop) = start:stop

function range_start_step_length(a::AbstractFloat, step::AbstractFloat, len::Integer)
range_start_step_length(promote(a, step)..., len)
end

# Range from start to stop: range(a, [step=s,] stop=b), no length
_range(start, step, stop, ::Nothing) = (:)(start, step, stop)
_range(start, ::Nothing, stop, ::Nothing) = (:)(start, stop)
function range_start_step_length(a::Real, step::AbstractFloat, len::Integer)
range_start_step_length(float(a), step, len)
end

# Range of a given length: range(a, [step=s,] length=l), no stop
_range(a::Real, ::Nothing, ::Nothing, len::Integer) = UnitRange{typeof(a)}(a, oftype(a, a+len-1))
_range(a::AbstractFloat, ::Nothing, ::Nothing, len::Integer) = _range(a, oftype(a, 1), nothing, len)
_range(a::AbstractFloat, st::AbstractFloat, ::Nothing, len::Integer) = _range(promote(a, st)..., nothing, len)
_range(a::Real, st::AbstractFloat, ::Nothing, len::Integer) = _range(float(a), st, nothing, len)
_range(a::AbstractFloat, st::Real, ::Nothing, len::Integer) = _range(a, float(st), nothing, len)
_range(a, ::Nothing, ::Nothing, len::Integer) = _range(a, oftype(a-a, 1), nothing, len)
function range_start_step_length(a::AbstractFloat, step::Real, len::Integer)
range_start_step_length(a, float(step), len)
end

_range(a::T, step::T, ::Nothing, len::Integer) where {T <: AbstractFloat} =
function range_start_step_length(a::T, step::T, len::Integer) where {T <: AbstractFloat}
_rangestyle(OrderStyle(T), ArithmeticStyle(T), a, step, len)
_range(a::T, step, ::Nothing, len::Integer) where {T} =
end

function range_start_step_length(a::T, step, len::Integer) where {T}
_rangestyle(OrderStyle(T), ArithmeticStyle(T), a, step, len)
end

_rangestyle(::Ordered, ::ArithmeticWraps, a::T, step::S, len::Integer) where {T,S} =
StepRange{typeof(a+zero(step)),S}(a, step, a+step*(len-1))
_rangestyle(::Any, ::Any, a::T, step::S, len::Integer) where {T,S} =
StepRangeLen{typeof(a+zero(step)),T,S}(a, step, len)

# Malformed calls
_range(start, step, ::Nothing, ::Nothing) = # range(a, step=s)
throw(ArgumentError("At least one of `length` or `stop` must be specified"))
_range(start, ::Nothing, ::Nothing, ::Nothing) = # range(a)
throw(ArgumentError("At least one of `length` or `stop` must be specified"))
_range(::Nothing, ::Nothing, ::Nothing, ::Nothing) = # range(nothing)
throw(ArgumentError("At least one of `length` or `stop` must be specified"))
_range(start::Real, step::Real, stop::Real, length::Integer) = # range(a, step=s, stop=b, length=l)
throw(ArgumentError("Too many arguments specified; try passing only one of `stop` or `length`"))
_range(::Nothing, ::Nothing, ::Nothing, ::Integer) = # range(nothing, length=l)
throw(ArgumentError("Can't start a range at `nothing`"))
range_start_step_stop(start, step, stop) = start:step:stop

function range_error(start, step, stop, length)
hasstart = start !== nothing
hasstep = step !== nothing
hasstop = stop !== nothing
haslength = start !== nothing

hint = if hasstart && hasstep && hasstop && haslength
"Try specifying only three arguments"
elseif !hasstop && !haslength
"At least one of `length` or `stop` must be specified."
elseif !hasstep && !haslength
"At least one of `length` or `step` must be specified."
elseif !hasstart && !hasstop
"At least one of `start` or `stop` must be specified."
else
"Try specifying more arguments."
end

msg = """
Cannot construct range from arguments:
start = $start
step = $step
stop = $stop
length = $length
$hint
"""
throw(ArgumentError(msg))
end

## 1-dimensional ranges ##

Expand Down Expand Up @@ -429,13 +509,13 @@ function LinRange(start, stop, len::Integer)
LinRange{T}(start, stop, len)
end

function _range(start::T, ::Nothing, stop::S, len::Integer) where {T,S}
function range_start_stop_length(start::T, stop::S, len::Integer) where {T,S}
a, b = promote(start, stop)
_range(a, nothing, b, len)
range_start_stop_length(a, b, len)
end
_range(start::T, ::Nothing, stop::T, len::Integer) where {T<:Real} = LinRange{T}(start, stop, len)
_range(start::T, ::Nothing, stop::T, len::Integer) where {T} = LinRange{T}(start, stop, len)
_range(start::T, ::Nothing, stop::T, len::Integer) where {T<:Integer} =
range_start_stop_length(start::T, stop::T, len::Integer) where {T<:Real} = LinRange{T}(start, stop, len)
range_start_stop_length(start::T, stop::T, len::Integer) where {T} = LinRange{T}(start, stop, len)
range_start_stop_length(start::T, stop::T, len::Integer) where {T<:Integer} =
_linspace(float(T), start, stop, len)
## for Float16, Float32, and Float64 we hit twiceprecision.jl to lift to higher precision StepRangeLen
# for all other types we fall back to a plain old LinRange
Expand Down
4 changes: 2 additions & 2 deletions base/twiceprecision.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ end
step(r::StepRangeLen{T,TwicePrecision{T},TwicePrecision{T}}) where {T<:AbstractFloat} = T(r.step)
step(r::StepRangeLen{T,TwicePrecision{T},TwicePrecision{T}}) where {T} = T(r.step)

function _range(a::T, st::T, ::Nothing, len::Integer) where T<:Union{Float16,Float32,Float64}
function range_start_step_length(a::T, st::T, len::Integer) where T<:Union{Float16,Float32,Float64}
start_n, start_d = rat(a)
step_n, step_d = rat(st)
if start_d != 0 && step_d != 0 &&
Expand Down Expand Up @@ -591,7 +591,7 @@ end
## LinRange

# For Float16, Float32, and Float64, this returns a StepRangeLen
function _range(start::T, ::Nothing, stop::T, len::Integer) where {T<:IEEEFloat}
function range_start_stop_length(start::T, stop::T, len::Integer) where {T<:IEEEFloat}
len < 2 && return _linspace1(T, start, stop, len)
if start == stop
return steprangelen_hp(T, start, zero(T), 0, len, 1)
Expand Down
19 changes: 19 additions & 0 deletions test/ranges.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,24 @@
# This file is a part of Julia. License is MIT: https://julialang.org/license

@testset "range costruction" begin
jw3126 marked this conversation as resolved.
Show resolved Hide resolved
@testset "range(;kw...)" begin
@test_throws ArgumentError range(start=1, step=1, stop=2, length=10)
@test_throws ArgumentError range(start=1, step=1, stop=10, length=11)

r = 3.0:2:11
@test r == range(start=first(r), step=step(r), stop=last(r) )
@test r == range(start=first(r), step=step(r), length=length(r))
@test r == range(start=first(r), stop=last(r), length=length(r))
@test r == range( step=step(r), stop=last(r), length=length(r))

r = 4:9
@test r === range(start=first(r), stop=last(r) )
@test r === range(start=first(r), length=length(r))
@test r == range(start=first(r), stop=last(r), length=length(r))
@test r === range( stop=last(r), length=length(r))
end
end

using Dates, Random
isdefined(Main, :PhysQuantities) || @eval Main include("testhelpers/PhysQuantities.jl")
using .Main.PhysQuantities
Expand Down