Implement accumulate and friends for arbitrary iterators
#34656
Conversation
|
|
||
| function iterate(itr::Accumulate) | ||
| state = iterate(itr.itr) | ||
| if state === nothing | ||
| return nothing | ||
| end | ||
| return (state[1], state) | ||
| val = Base.BottomRF(itr.f)(itr.init, state[1]) |
There was a problem hiding this comment.
Since Base.BottomRF(itr.f) calls reduce_first, this patch changes the output:
Before:
julia> collect(Iterators.accumulate(+, (x for x in [true])))
1-element Array{Bool,1}:
1
julia> collect(Iterators.accumulate(+, (x for x in [true, true, false])))
3-element Array{Integer,1}:
true
2
2After:
julia> collect(Iterators.accumulate(+, (x for x in [true])))
1-element Array{Int64,1}:
1
julia> collect(Iterators.accumulate(+, (x for x in [true, true, false])))
3-element Array{Int64,1}:
1
2
2Ref:
Lines 73 to 78 in 3720edf
|
Bump. Can this be reviewed and merged? (The test failure in macOS looks like something irrelevant from |
| @test cumsum(x^2 for x in 1:3) == [1, 5, 14] | ||
| @test cumprod(x + 1 for x in 1:3) == [2, 6, 24] | ||
| @test accumulate(+, (x^2 for x in 1:3); init=100) == [101, 105, 114] | ||
| end |
There was a problem hiding this comment.
Maybe add a test for Iterators.accumulate taking an init keyword?
There was a problem hiding this comment.
Some more tests are added in 78a6c44
|
Nice addition! I was missing this functionality today. I will merge in a couple of days if no objections (please @tkf ping me if I forget). |
|
ping :) |
| @@ -250,6 +262,10 @@ julia> accumulate(+, fill(1, 3, 3), dims=2) | |||
| ``` | |||
| """ | |||
| function accumulate(op, A; dims::Union{Nothing,Integer}=nothing, kw...) | |||
There was a problem hiding this comment.
Sorry that I missed that on first review: wouldn't this function need a compat annotation, like cumsum and cumprod ?
There was a problem hiding this comment.
Thanks for catching it. I added it in 7a4d7fe.
There was a problem hiding this comment.
Thanks, in the meantime I also thought about compat for the iterator version, and was sorry to have to ask you again another change, but you already made it :)
|
As this changes very slightly the result of some accumulate operations (cf https://github.com/JuliaLang/julia/pull/34656/files#r374510231), I will apply the "minor change" label, but this migth be too minor to deserve it. |
rfourquet
added
the
minor change
This PR implements
Base.accumulate(op, itr)simply by wrappingIterators.accumulate. I addedinitkeyword argument toIterators.accumulateto match the call signature.I'm posting this as a WIP PR as it is a patch on top #34654 which needs to go first.(Edit: ready for review now)