Add fallback shuffle method: shuffle(itr) = shuffle(collect(itr))

[CameronBieganek]

Currently, shuffle does not work on iterators:

julia> itr = Iterators.product(1:3, 4:6); collect(itr)
3×3 Array{Tuple{Int64,Int64},2}:
 (1, 4)  (1, 5)  (1, 6)
 (2, 4)  (2, 5)  (2, 6)
 (3, 4)  (3, 5)  (3, 6)

julia> shuffle(itr)
ERROR: MethodError: no method matching shuffle(::Base.Iterators.ProductIterator{Tuple{UnitRange{Int64},UnitRange{Int64}}})
Closest candidates are:
  shuffle(::AbstractRNG, ::AbstractArray) at /Users/julia/buildbot/worker/package_macos64/build/usr/share/julia/stdlib/v1.5/Random/src/misc.jl:251
  shuffle(::AbstractArray) at /Users/julia/buildbot/worker/package_macos64/build/usr/share/julia/stdlib/v1.5/Random/src/misc.jl:252
Stacktrace:
 [1] top-level scope at REPL[30]:1

julia> shuffle(collect(itr))
3×3 Array{Tuple{Int64,Int64},2}:
 (2, 5)  (3, 5)  (3, 4)
 (2, 6)  (1, 4)  (3, 6)
 (1, 6)  (2, 4)  (1, 5)

Since the only two shuffle methods are defined on AbstractArray, it seems like it would be handy to have a fallback definition for shuffle that first collects the iterator, like

shuffle(itr) = shuffle(collect(itr))

Maybe I've overlooked a reason why this shouldn't be implemented?

[KristofferC]

Maybe I've overlooked a reason why this shouldn't be implemented?

Should all methods that work on arrays have a fallback that calls collect? Personally, I think it is clear and straightforward that if you have an iterator and want to call a method only working on arrays, then you collect and call the method. Makes it explicit what is going on and that you actually have to allocate the array up front instead of hiding that.

[fredrikekre]

related: #37195

[CameronBieganek]

Yeah, that makes sense. I was thinking that it seems reasonable to shuffle an iterator, but I guess a more precise way to phrase the litmus test for whether foo should work on iterators (as opposed to my "seems reasonable" litmus test) is something like this:

Is it possible to write a foo(itr) method that returns an iterator?

If not, then there shouldn't be a fallback foo(itr) = foo(collect(itr)) method. And I suppose it's not possible to have shuffle return an iterator. 😅