-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
range(; stop) and range(; length). Single keyword args only. Single pos arg not allowed. #39241
Conversation
* Only `start` and `stop` are provided | ||
* Only `length` and `stop` are provided | ||
|
||
A `UnitRange` is not produced if `step` is provided even if specified as one. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the whole extended help section, it is very clear.
aea441e
to
bd6cd3c
Compare
eb01a80
to
358d1a4
Compare
358d1a4
to
9daf90a
Compare
@mbauman, could we add the triage tag to this pull request? This request just adds two keyword forms where only Single positional argument should be its own pull requestAdding a single positional argument on top of this could be as easy as
julia> Base.range(stop) = Base.range_stop(stop)
julia> methods(range)
# 4 methods for generic function "range":
[1] range(; start, stop, length, step) in Base at range.jl:138
[2] range(stop; stop, length, step) in Main at REPL[5]:1
... You could also dispatch from within the positional argument versions of the function if Single positional argument Avoiding the ambiguity of whether
|
Speaking personally, I'm rather lukewarm about these final steps, and this hasn't gotten much engagement from others. There's a tradeoff here in the complexity of usage and documentation, and I don't see the new functionality significantly outweighing these costs. I'd be inclined to let the range changes simmer a bit — let's live with the new functionality for a release or two and see how it feels before doing more. |
That's fine with me. I was under the impression that we did not want space this out over many releases. |
Since julia> Base.OneTo(3) isa UnitRange
false It's worth to consider all the places in julia as well as in the ecosystem where method arguments are restricted to
Changing the return type of |
I wouldn't consider that a show-stopper. This isn't changing any return types. It's enabling new methods that have new behaviors. Even if it did change return types, I wouldn't necessarily consider it a show stopper; it'd just be one more thing to weigh the new functionality against. |
That's an important consideration. Note that julia> UnitRange <: AbstractUnitRange
true
julia> Base.OneTo <: AbstractUnitRange
true
julia> UnitRange(Base.OneTo(3))
1:3
julia> convert(UnitRange, Base.OneTo(5))
1:5
julia> methodswith(AbstractUnitRange)
[1] getindex(r::AbstractUnitRange, s::AbstractUnitRange{var"#s91"} where var"#s91"<:Integer) in Base at range.jl:695
[2] getindex(r::AbstractUnitRange, s::StepRange{var"#s91",S} where S where var"#s91"<:Integer) in Base at range.jl:709
[3] getindex(A::SparseArrays.AbstractSparseMatrixCSC{Tv,Ti} where Ti<:Integer, I::AbstractUnitRange) where Tv in SparseArrays at C:\Users\kittisopikulm\AppData\Local\Programs\Julia 1.5.0\share\julia\stdlib\v1.5\SparseArrays\src\sparsevector.jl:626
[4] getindex(x::SparseArrays.AbstractSparseMatrixCSC, I::AbstractUnitRange, j::Integer) in SparseArrays at C:\Users\kittisopikulm\AppData\Local\Programs\Julia 1.5.0\share\julia\stdlib\v1.5\SparseArrays\src\sparsevector.jl:531
[5] getindex(x::SparseArrays.AbstractSparseArray{Tv,Ti,1}, I::AbstractUnitRange) where {Tv, Ti} in SparseArrays at C:\Users\kittisopikulm\AppData\Local\Programs\Julia 1.5.0\share\julia\stdlib\v1.5\SparseArrays\src\sparsevector.jl:784
[6] checkindex(::Type{Bool}, inds::AbstractUnitRange, i::Real) in Base at abstractarray.jl:563
[7] checkindex(::Type{Bool}, inds::AbstractUnitRange, ::Colon) in Base at abstractarray.jl:564
[8] checkindex(::Type{Bool}, inds::AbstractUnitRange, ::Base.Slice) in Base at abstractarray.jl:565
[9] checkindex(::Type{Bool}, inds::AbstractUnitRange, r::AbstractRange) in Base at abstractarray.jl:566
[10] checkindex(::Type{Bool}, indx::AbstractUnitRange, I::Base.LogicalIndex) in Base at multidimensional.jl:695
[11] checkindex(::Type{Bool}, indx::AbstractUnitRange, I::AbstractArray{Bool,1}) in Base at abstractarray.jl:570
[12] checkindex(::Type{Bool}, indx::AbstractUnitRange, I::AbstractArray{Bool,N} where N) in Base at abstractarray.jl:571
[13] checkindex(::Type{Bool}, inds::AbstractUnitRange, I::AbstractArray) in Base at abstractarray.jl:572
[14] checkindex(B::Type{Bool}, inds::AbstractUnitRange, i::StaticArrays.StaticIndexing{T}) where T in StaticArrays at C:\Users\kittisopikulm\.julia\packages\StaticArrays\LJQEe\src\indexing.jl:368
[15] checkindex(::Type{Bool}, inds::AbstractUnitRange, i) in Base at abstractarray.jl:561
[16] findfirst(p::Union{Base.Fix2{typeof(==),T}, Base.Fix2{typeof(isequal),T}}, r::AbstractUnitRange) where T<:Integer in Base at array.jl:1867
[17] isempty(r::AbstractUnitRange) in Base at range.jl:503
[18] issorted(r::AbstractUnitRange) in Base at range.jl:1011
[19] length(r::AbstractUnitRange) in Base at range.jl:545
[20] maximum(r::AbstractUnitRange) in Base at range.jl:602
[21] minimum(r::AbstractUnitRange) in Base at range.jl:601
[22] sort(r::AbstractUnitRange) in Base at range.jl:1014
[23] sort!(r::AbstractUnitRange) in Base at range.jl:1015
[24] sortperm(r::AbstractUnitRange) in Base at range.jl:1019
[25] step(r::AbstractUnitRange{T}) where T in Base at range.jl:528
[26] view(r1::AbstractUnitRange, r2::AbstractUnitRange{var"#s91"} where var"#s91"<:Integer) in Base at subarray.jl:167
[27] view(r1::AbstractUnitRange, r2::StepRange{var"#s91",S} where S where var"#s91"<:Integer) in Base at subarray.jl:171 |
Just an idea: What about allowing |
There's currently no method that matches
So defining Just a quick test:
|
That's an interesting thought and could be easily implemented: julia> (::Colon)(::typeof(one), x::Integer) = Base.OneTo(x)
julia> one:5
Base.OneTo(5) I would expect the discussion of that to go along the lines of #39242 . It is also distinct from the objectives of this PR. I encourage you to submit a new PR or issue to propose this. |
Looking at these again, I don't really have any objection: they all fall out of the single assumption that the default |
The other assumption is that Note that this PR does not allow for a Pythonic |
Right. Last time around there were enough of these |
The first method in that list was generalized to |
I'm not sure if there's already an issue on that - the range-from-one and range-from-zero could be done very elegantly if we had precision-independent representations for one and zero in the language, with separate types for one and zero (along the lines of https://github.com/perrutquist/Zeros.jl), like we have with This approach has worked nicely for constants like |
I think the concept of Zeros.jl sounds great, but I'm quite unclear how range-from-one or range-from-zero might be done elegantly from that. Using types already in Base there are the method types Anyways, modifying |
I have resolved the conflicts that were present due to other changes on the master branch. The only failing test is for Also I have looked into potential issues (#41805, #41807 and #41809) with using |
If we had special types for one and zero, let's call them Zero():n == Base.ZeroTo(n)
One():n == Base.OneTo(n) (We currently don't have That would be even cleaner than |
juila> import Base: one, zero
julia> (::Colon)(::typeof(zero), n) = zero(n):n
julia> (::Colon)(::typeof(one), n) = Base.OneTo(n)
julia> zero:5
0:5
julia> one:3
Base.OneTo(3) |
Sure, |
Oh, sure! |
As a single keyword arg only (single positional arg not allowed still)
As a single keyword arg only (single positional arg not allowed still)
range(; stop), range(; length)
This is a minimalist version of #39223 to expose
Base.OneTo
when a single keyword argument ofrange
is given. It only allows eitherstop
orlength
to be provided as the sole keyword argument. In that circumstance only,start
, andstep
are assumed to be one.It does not allow for one positional argument to be given.
Usage:
Background on Base.OneTo
Base.OneTo
is a highly optimized version of1:n
that uses the type system to guarantee the first element is one. While1:n
is very common in Julia, it should be replaced byBase.OneTo(n)
in highly optimized code. This pull request attempts to make it more accessible viarange
.References
This strictly implements