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.

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

Doc PR to clarify range usage #37875

Closed
wants to merge 10 commits into from
Closed

Doc PR to clarify range usage #37875

wants to merge 10 commits into from

Conversation

mkitti
Copy link
Contributor

@mkitti mkitti commented Oct 3, 2020

The current documentation for range is at worse wrong and at best misleading. This documentation change clarifies the usage of range.

The current documentation for `range` is at worse wrong and at best misleading. This documentation change clarifies the usage of `range`.

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.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If length, stop, and step are all specified, they must agree.

length, stop, and step cannot be all specified:

julia/base/range.jl

Lines 166 to 167 in 01d89c6

_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(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(start[, stop]; length, stop, step=1)

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)).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

step does not default to 1 when length is specified.

step does not default to 1 when stop is a positional argument. range(1,5) does not work but range(1,5, step=1) does.

The only time step defaults to 1 is when using stop as a positional argument: range(1,stop=5)

* `start`, `length`, and `stop` must be integers to get a `UnitRange`
* A `step` of 1 does not result in a `UnitRange`
base/range.jl Outdated Show resolved Hide resolved
base/range.jl Outdated Show resolved Hide resolved
Copy link
Member

@dkarrasch dkarrasch left a comment

Choose a reason for hiding this comment

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

Much appreciated!

@dkarrasch dkarrasch added the docs This change adds or pertains to documentation label Oct 5, 2020
base/range.jl Outdated
Comment on lines 55 to 60
# The following cause an ArgumentError to be thrown.
range(start, stop) # Must specify either length or step
range(start; step) # Cannot specify step alone
range(start, stop; length, step) # Cannot specify all of stop, length, and step
range(start; stop, length, step) # Cannot specify all of stop, length, and step

Copy link
Member

Choose a reason for hiding this comment

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

I believe we should always put the one-or-two-line description right after the function signature.

This can be moved backward in the Examples section.

base/range.jl Outdated
Comment on lines 121 to 122
julia> try range(1, 5) catch e println(e) end
ArgumentError("At least one of `length` or `step` must be specified")
Copy link
Member

Choose a reason for hiding this comment

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

Alongside with the previous one comment, this could just be

# Must specify either length or step
julia> range(1, 5)
ERROR: ArgumentError: At least one of `length` or `step` must be specified

And the doctest only match the starts (if I get it correctly). For example, in OffsetArrays: https://github.com/JuliaArrays/OffsetArrays.jl/blob/87666ee004664282370122ba68e7467c13988b38/src/axes.jl#L12-L24

Reference: https://juliadocs.github.io/Documenter.jl/dev/man/doctests/#Exceptions

range doc: grammatical and formatting corrections

Co-authored-by: Daniel Karrasch <daniel.karrasch@posteo.de>
@mcabbott
Copy link
Contributor

mcabbott commented Oct 5, 2020

Could most of this be part of Extended Help, I think it's called, the lower section shown by ??range? I agree that the current docstring isn't great, I guess it grew over time, but IMO it should be possible to convey what it does in a few sentences & a few examples. I think getting to the examples sooner, not later, would be an improvement. I don't think a listing of ways to produce an ArgumentError needs to live before the description of what this basically does.

An attempt:

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

Construct at `UnitRange` or a `StepRange`, given the a starting value, and at least one keyword. 

If the only keyword is `stop` or `length`, then the default step is `1`. This results in a `UnitRange` 
if started at an integer. Otherwise it constructs a `StepRange`, for which any two of `stop, length, step` 
can be specified, and the third will be calculated. Giving all three will result in an error. 
(`stop` may be a keyword, or the 2nd argument.)

Special care is taken to ensure intermediate values are computed rationally. To avoid this
induced overhead, see the `LinRange` constructor.

  │ Julia 1.1
  │
  │  stop as a positional argument requires at least Julia 1.1.

## Examples


## Extended Help

@mkitti
Copy link
Contributor Author

mkitti commented Oct 5, 2020

A help / extended help split could work. I really have an issue with stating that step=1 in the function signature. step is not 1 in many cases and setting it to 1 does not result in the same behavior when it is specified. I did a quick edit as follows.

    range(start[, stop]; length, stop, step)

Construct at an `AbstractRange`, given a starting value, and at least one keyword argument.

`start` is the first value of range. `stop` is the upper bound of the range. `length` is the number of
elements of range. `step` is the separation between each element of the range.

If the only keyword is `stop` or `length` and all arguments are integers this results in a `UnitRange`.
Otherwise it constructs an `AbstractRange` for which any two of `stop, length, step` 
can be specified. Giving all three will result in an error. 
`stop` may be a keyword, or the 2nd positional argument along with another keyword argument.

  │ Julia 1.1
  │
  │  stop as a positional argument requires at least Julia 1.1.

## Examples


## Extended Help

StepRangeLen could also occur:

julia> typeof(1:0.2:3)
StepRangeLen{Float64,Base.TwicePrecision{Float64},Base.TwicePrecision{Float64}}

I wondering if specifying that is too much detail. I think we should only mention UnitRange and just say guarantee them to be a subtype of AbstractRange

I also removed the reference to the missing parameter being calculated. No additional calculations are necessarily done in many circumstances.

We should also minimally explain the keyword arguments.

@mcabbott
Copy link
Contributor

mcabbott commented Oct 5, 2020

I really have an issue with stating that step=1 in the function signature.

I guess there's no way to specify what this function does in one signature line, the rules are more complicated than that. In three lines you could say:

    range(start; stop, length, step) # any two keywords
    range(start, stop; length, step) # any one keyword
    range(start; stop, length) # any one keyword, has step=1

Construct at an `AbstractRange`, given a starting value, and at least one keyword argument.

But I suppose the spirit in which to take the existing line is that it isn't code, it's meant to be explained in english below, and exists mostly to remind you what the possible keywords are (e.g. does Julia use stop or last or final?) Writing all the detail right away sort-of nudges you to digest it before being told what the function is trying to achieve.

StepRangeLen could also occur:

Yes, my writing StepRange was a mistake, I agree with just AbstractRange / UnitRange, and maybe "step range" in words (to mean not-a-UnitRange).

I also removed the reference to the missing parameter being calculated. No additional calculations are necessarily done in many circumstances.

The core idea here is, I think, that there are 4 unknowns and 1 equation, so you have to provide 3 to uniquely specify the answer. If you provide 4, there is almost always no solution, hence the error. Maybe the docstring should write the equation somewhere: stop == start + step * length. These 4 aren't all fields of all (any?) type, sometimes calculation is deferred until you say length(1:5), but that's a detail.

@mcabbott
Copy link
Contributor

mcabbott commented Oct 5, 2020

How about this, to stress the 4 unknowns view, and keep things compact: [Edited]

    range(start; stop, length, step) # any two keywords
    range(start, stop; length, step) # any one keyword
    range(start; stop, length) # any one keyword, has step=1

Construct at an `AbstractRange`, given a starting value, and exactly two of the other unknowns. 

The method with implicit `step=1` will create a `UnitRange` when all arguments are integers.
This can also be constructed `start:stop`.

Otherwise this always returns some type of step range. 
Like `start:step:stop`, it will use the given `step` but may stop short of the given `stop`.
For floating-point numbers, special care is taken to ensure that intermediate values are computed rationally.
To avoid this overhead, see the `LinRange` constructor.

  │ Julia 1.1
  │
  │  stop as a positional argument requires at least Julia 1.1.

## Examples

@mkitti
Copy link
Contributor Author

mkitti commented Oct 5, 2020

and exactly two of the other unknowns in stop == start + step * length.

That's not accurate. start + step * ( length - 1) <= stop is more like it.

I incorporated your three line proposal.

@mcabbott
Copy link
Contributor

mcabbott commented Oct 5, 2020

Oh right, stop ≈ start + step * (length-1) is closer... but I forgot about this:

julia> range(1, stop=3.5)
1.0:1.0:3.0

range(start; stop, length) # any one keyword, creates a UnitRange (step = 1)

This is not true, e.g. range(1.0; stop=5).

@mkitti
Copy link
Contributor Author

mkitti commented Oct 5, 2020

range(start; stop, length) # any one keyword, creates a UnitRange (step = 1)

I removed the mention of UnitRange in the beginning.

I structured the Extended help section and added the equation express there.

@mkitti
Copy link
Contributor Author

mkitti commented Jan 8, 2021

Looks like range will change before this is merged and this PR is obsolete. After the all positional range and all keyword range pull requests are merged, I'll revisit this and see if any of the documentation is worth salvaging.

Thank you, @dkarrasch, @johnnychen94, and @mcabbott for helping to put this together.

@mkitti
Copy link
Contributor Author

mkitti commented Jan 8, 2021

Closing due to pending changes in #38041

@mkitti mkitti closed this Jan 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants