-
-
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
RFC: use first/last as endpoints for Range indexing #15750
Conversation
@@ -210,15 +210,15 @@ index_shape_dim(A, dim, ::Colon) = (trailingsize(A, dim),) | |||
# ambiguities for AbstractArray subtypes. See the note in abstractarray.jl | |||
|
|||
# Note that it's most efficient to call checkbounds first, and then to_index | |||
@inline function _getindex(l::LinearIndexing, A::AbstractArray, I::Union{Real, AbstractArray, Colon}...) | |||
@inline function _getindex(l::LinearIndexing, A::AbstractArray, I::Union{Real, AbstractArray, Colon, EndpointRange}...) |
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.
Shouldn't be necessary if EndpointRange <: Range{Int}
.
Clever. Another thought I've had here is to parse I do imagine that the majority of uses of |
Arithmetic is also doable (e.g., add fields |
Offsets are obviously quite common. If you're using |
Other methods I've used or seen are Fast anonymous functions are amazing: julia> Base.getindex(A::AbstractArray, f::Function) = A[f(length(A))]
julia> f(A) = A[(x)->x-1]
g(A) = A[end-1];
julia> @benchmark f(1:10)
================ Benchmark Results ========================
Time per evaluation: 5.69 ns [5.59 ns, 5.79 ns]
Proportion of time in GC: 0.00% [0.00%, 0.00%]
Memory allocated: 0.00 bytes
Number of allocations: 0 allocations
Number of samples: 5401
Number of evaluations: 344501
R² of OLS model: 0.956
Time spent benchmarking: 6.15 s
julia> @benchmark g(1:10)
================ Benchmark Results ========================
Time per evaluation: 5.80 ns [5.72 ns, 5.89 ns]
Proportion of time in GC: 0.00% [0.00%, 0.00%]
Memory allocated: 0.00 bytes
Number of allocations: 0 allocations
Number of samples: 7401
Number of evaluations: 2314101
R² of OLS model: 0.957
Time spent benchmarking: 8.34 s |
Indeed the more flexible implementation would be through building up anonymous functions. That should be able to handle any expression. Then the key question is whether it's OK/attractive to (ab)use |
the current docs say this about first, last: endof gives the index used with last when last is used applicatively. |
Good point about the distinction between value and index. One of the other notions I'm contemplating is introducing an Of course, perhaps it would be better to call it If we introduce that, then I think If instead we keep |
I love the fact that you dispatch on function types for this, @timholy! That said, I would really hope to keep this final choice of names simple
(assuming, of course, that We do have a Cheers! On Monday, April 4, 2016, Tim Holy notifications@github.com wrote:
|
I agree there should only be one way to do it, so if we find a replacement here, Dispatch on function types here is cute, but I'm afraid it's too cute. Why should |
Thanks, @kmsquire. And thanks, @mbauman, for writing almost word-for-word the post I would have written. However, rather than the implementation in your link, I think that (now) the better approach is immutable IndexFunction{F<:Function}
index::F
end
IndexFunction(::First) = IndexFunction((A,d) -> first(inds(A,d)))
IndexFunction(::Last) = IndexFunction((A,d) -> last(inds(A,d)))
(+)(m::Union{First,Last}, i::Number) = IndexFunction(m)+i
(+)(m::IndexFunction, i::Number) = IndexFunction((A,d) -> m.index(A,d)+i)
... etc. We could let Either way, I think this is fully composable, and it uses anonymous functions rather than reimplementing them. It does require that you define all the operations in #15750 (comment). |
Yes, that works. I guess my point is that it's not all that different from a terse lambda notation (#5571 (comment)). Instead of using syntax and precedence to solve the closure expansion issue, you must manually define the functions that it can expand through. It seems fiddly (and maybe has ambiguity issues), but the proposed rules in 5571 are fiddly, too. |
Are you proposing to instead implement these operations via special parsing rules? Or pointing out that a parser-free solution shares similar, significant downsides to that example? I'm not thrilled either, but I don't see a solution different from "lean on the parser even harder than we have so far" (get the parsing working outside of |
I point to it because it feels like the same problem to me. We want to be able to express this: sub(A, (ifirst, ilast) -> ifirst+1:ilast, (ifirst, ilast) -> setdiff(ifirst:ilast, idxs), (ifirst, ilast) -> [ifirst,ilast]) in a terse, easy-to-use, and easy-to-understand manner. I suppose the set of operations is limited enough that we'll be able to cover 99.9% of uses with your inside-out approach, but I don't see a path forward with |
One further option: That's back to "leaning on the parser"; we'd want to add |
either first,last xor begin,end xor start,finish .. (imo): |
While yes using For example, it is very common in machine learning to need to divide a dataset into test, validation and train subsets. This currently looks like:
(Remembering that Removing |
Closing; experimentation underway at https://github.com/timholy/EndpointRanges.jl. |
My vote here would be to reuse the While a type here like in EndPointRanges is really quite cool, the great thing about |
That's aligned with my current thinking. The only negative is that you can't easily extract the indices independently of the array itself. (We can't do that for conventional arrays either, of course.) |
|
I'm in support. We get to simplify some of the parser rules in this release since we no longer need Maybe extend
|
Yes, I suspect that's the best way to proceed. |
For reference of anyone who doesn't know (since apparently this hasn't been mentioned anywhere in this thread?).
Currently, in the basic |
That's interesting. For StaticArrays I feel something like |
This is basically a trial balloon to see what folks think of some thoughts I've had on handling syntax challenges for certain parts of #15648.
Demo:
Compared to
a[2:end]
,a[2:last]
does not require special parser tricks, and2:last
has independent existence and can be passed as an argument to a function.