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

Add range(start, stop) and range(start, stop, length) #39228

Merged
merged 9 commits into from
Feb 4, 2021

Conversation

antoine-levitt
Copy link
Contributor

Fix #38750

I implemented range(start, stop) as that seemed simpler to document and be more consistent with the main docs. I can leave it undefined as it was before if people prefer.

base/range.jl Outdated Show resolved Hide resolved
NEWS.md Outdated Show resolved Hide resolved
base/range.jl Outdated Show resolved Hide resolved
base/range.jl Outdated Show resolved Hide resolved
Copy link
Member

@mbauman mbauman left a comment

Choose a reason for hiding this comment

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

Note that this simplifies both documentation and implementation. 👍

The CI error is:

@test_throws ArgumentError range(1, 100)

base/range.jl Outdated Show resolved Hide resolved
test/ranges.jl Outdated
Comment on lines 17 to 21
@test r === range( stop=last(r), length=length(r))
@test r === range(first(r), last(r))
# the next ones use ==, because it changes the eltype
@test r == range(first(r), last(r), length(r))
@test r == range(start=first(r), stop=last(r), length=length(r))
Copy link
Contributor

@mkitti mkitti Jan 15, 2021

Choose a reason for hiding this comment

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

Suggested change
@test r === range( stop=last(r), length=length(r))
@test r === range(first(r), last(r))
# the next ones use ==, because it changes the eltype
@test r == range(first(r), last(r), length(r))
@test r == range(start=first(r), stop=last(r), length=length(r))
@test r === range( stop=last(r), length=length(r))
@test r === range( first(r), last(r) )
# the next ones use ==, because it changes the eltype
@test r == range( first(r), last(r), length(r))
@test r == range(start=first(r), stop=last(r), length=length(r))

@jw3126 did a nice job of lining up similar aspects from each line. Perhaps we should maintain that style.

@jw3126
Copy link
Contributor

jw3126 commented Jan 16, 2021

While I am in favor of this, it is controversal:
#38041 (comment)
#28708 (comment)

@mkitti
Copy link
Contributor

mkitti commented Jan 16, 2021

While I am in favor of this, it is controversal:
#38041 (comment)
#28708 (comment)

Yes, but it's been discussed recently. See #38750 (comment)

@mkitti
Copy link
Contributor

mkitti commented Jan 18, 2021

I just noticed that the tests you wrote are in a testset titled "range(;kw...)"

@antoine-levitt
Copy link
Contributor Author

Thanks, I just removed the testset.

@mkitti
Copy link
Contributor

mkitti commented Jan 21, 2021

Merge conflict due to #39283

@mbauman mbauman merged commit 4af2b9f into JuliaLang:master Feb 4, 2021
ElOceanografo pushed a commit to ElOceanografo/julia that referenced this pull request May 4, 2021
* Add range(start, stop) and range(start, stop, length)

Co-authored-by: Matt Bauman <mbauman@juliacomputing.com>
antoine-levitt added a commit to antoine-levitt/julia that referenced this pull request May 9, 2021
* Add range(start, stop) and range(start, stop, length)

Co-authored-by: Matt Bauman <mbauman@juliacomputing.com>
matthias314 added a commit to matthias314/julia that referenced this pull request Jun 11, 2021
* added comments and reference for zero-dimensional arrays

* updated table entry for `range` to reflect JuliaLang#39228

* made spelling of "one-dimensional" etc. uniform throughout the documentation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

range(start, stop, length)
4 participants