JuliaFolds / Transducers.jl

Efficient transducers for Julia
https://juliafolds.github.io/Transducers.jl/dev/
MIT License
433 stars 24 forks source link

`Zip` broken? #562

Open ParadaCarleton opened 12 months ago

ParadaCarleton commented 12 months ago
julia> Zip(collect(1:10), collect(0:9)) |> collect
ERROR: MethodError: no method matching length(::Transducers.Composition{Map{typeof(Transducers._zip_init)}, Transducers.Composition{Transducers.ZipSource{ComposedFunction{Vector{Int64}, Map{typeof(first)}}}, Transducers.Composition{Map{typeof(Transducers._zip_between)}, Transducers.Composition{Transducers.ZipSource{ComposedFunction{Vector{Int64}, Map{typeof(first)}}}, Transducers.Composition{Map{typeof(Transducers._zip_between)}, Map{typeof(last)}}}}}})

Closest candidates are:
  length(::ExponentialBackOff)
   @ Base error.jl:267
  length(::Highlights.Format.TokenIterator)
   @ Highlights ~/.julia/packages/Highlights/AOis2/src/Format.jl:170
  length(::Core.SimpleVector)
   @ Base essentials.jl:765
  ...

Stacktrace:
 [1] _similar_shape(itr::Transducers.Composition{Map{…}, Transducers.Composition{…}}, ::Base.HasLength)
   @ Base ./array.jl:708
 [2] _collect(cont::UnitRange{…}, itr::Transducers.Composition{…}, ::Base.HasEltype, isz::Base.HasLength)
   @ Base ./array.jl:763
 [3] collect(itr::Transducers.Composition{Map{…}, Transducers.Composition{…}})
   @ Base ./array.jl:757
 [4] |>(x::Transducers.Composition{Map{…}, Transducers.Composition{…}}, f::typeof(collect))
   @ Base ./operators.jl:915
 [5] top-level scope
   @ REPL[111]:1
Some type information was truncated. Use `show(err)` to see complete types.

I'm actually not sure how Zip is supposed to work, if things that work with zip error.

artemsolod commented 11 months ago

Zip expects Transducers in its arguments and gives a new combined transducer. So the use case is something like this

1:10 |> Zip(Map(x -> x + 1), Map(x -> x - 1)) |> collect

that is is starting with data, we feed it into a transducer and finally collect results.

I am not sure if Transducers has its own way to zip sources but plain zip from Base may work for you

zip(1:10, 0:9) |> Map(sum) |> collect
ParadaCarleton commented 11 months ago

Zip expects Transducers in its arguments and gives a new combined transducer. So the use case is something like this

OK, so in that case, it probably makes sense to error when we call Zip (I'm guessing the arguments for Zip don't have type annotations), and that should probably be reflected in the documentation. (Possibly even in a new/different name and deprecating Zip?)

I think it might make sense to just allow generic collections as well, since they correspond to the degenerate case where we're ignoring the input.