-
-
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
Implement getindex
, firstindex
, and lastindex
for Generators
#48745
Comments
Also ref #36175 |
Reading through those, I see lots of support for this proposal. The motivation for pr #34678 is issue #34674, in which we find:
The same argument applies here to As you point out, in #34678 (comment) @tkf objected:
Despite that objection, #34678 was merged anyway; this proposal would help resolve that conflict. Further, in #36175, @tkf says:
This proposal is consistent with that intent, and we could probably implement P.S. As seen in #48510, I think also |
For multidimensional generators, it looks like indexing would need something like this too: Base.getindex(itr::Iterators.ProductIterator, args...) = ntuple(length(args)) do i; itr.iterators[i][args[i]] end |
Is it possible to know that a generator doesn't have side effects? E.g, what about something like julia> a = Int[];
julia> g = (push!(a, i) for i in 1:4)
Base.Generator{UnitRange{Int64}, var"#5#6"}(var"#5#6"(), 1:4) With this, julia> g[2]
1-element Vector{Int64}:
2
julia> g[2]
2-element Vector{Int64}:
2
2 |
Note that we already observe this with functions with side-effects, julia> a = Int[]
g = (push!(a, i) for i=1:4)
@show last(g)
@show last(g);
last(g) = [4]
last(g) = [4, 4] so this proposal adds no new surprise; I think the answer is, "know thy function and know thy iterator." |
A generator like this:
should be indexable:
especially considering that
eachindex(::Generator)
is implemented.Currently, generators implement
last
but notlastindex
:This contradicts the user manual:
So, to be consistent with
eachindex
,first
, andlast
, generators should also implementgetindex
,firstindex
, andlastindex
.The text was updated successfully, but these errors were encountered: