`eachindex` docstring is wrong; it doesn't accept arbitrary iterables

[adienes]

help?> eachindex
search: eachindex eachline lastindex checkindex getindex firstindex

  eachindex(A...)


  **In general eachindex accepts arbitrary iterables, including strings and
  dictionaries,** 

but

julia> d = Dict([:a=>1, :b=>-1])
Dict{Symbol, Int64} with 2 entries:
  :a => 1
  :b => -1

julia> eachindex(d, d)
ERROR: MethodError: no method matching keys(::Dict{Symbol, Int64}, ::Dict{Symbol, Int64})

the docstring should clarify that only the one-arg method supports arbitrary iterables

[jishnub]

This seems to be an implementation issue that may be fixed by mapping keys over the arguments?

[adienes]

I don't understand. what would I do with the result of mapping keys over the arguments, take the intersection? but why not use eachindex recursively for this purpose

[jakobnissen]

I object to the idea that arbitrary iterators even have indices. What's the index of a generator? What about eachindex(Iterators.filter(isodd, (i for i in 1:3))? It doesn't even support Set.

eachindex doesn't have a generic definition, and it can't have. It would be more correct to write that indexable containers should implement eachindex.

[adienes]

I agree (as in github.com//issues/58341) that the 1-arg method itself is pretty objectionable. I guess I intended this issue for the fact that the vararg-arity claimed in the docstring doesn't exist either. but I would definitely prefer solutions to look more like removing (or at least narrowing) claims from the docs rather than adding methods

[mbauman]

There is a generic implementation, it's just not doing what (I suspect) the author intended.

julia/base/abstractarray.jl

Line 318 in df91352

eachindex(itrs...) = keys(itrs...)

That should be:

eachindex(itrs...) = map(keys, itrs)

And then the doc string should state "arbitrary indexables" instead of iterables.

[jakobnissen]

It should not map I believe - then it would work very differently from what it does on arrays. For arrays it checks all arguments have the same keys, and then returns a single iterator over those keys (whereas map would return a tuple of iterators over each iterable's keys). It doesn't say what it does when passed multiple non-arrays.

If eachindex should support multiple non-arrays, then I think it should work the same. However, I don't see how this can be done generically, so it's better to delete that fallback method. Is it used anywhere?

There is also another mistake in the docs. The docstring promises

If you supply more than one AbstractArray argument, eachindex will create an
iterable object that is fast for all arguments (typically a UnitRange if all
inputs have fast linear indexing, a CartesianIndices otherwise). If the
arrays have different sizes and/or dimensionalities, a DimensionMismatch
exception will be thrown.

However

julia> eachindex(zeros(2, 2), zeros(4)) # different size and dimensionality
Base.OneTo(4)

I'm not sure what is meant by the "and/or" in that paragraph, but that result sure does look like it contradicts the docstring.

[adienes]

don't forget eachindex(ntuple(identity, 4), ntuple(identity, 5)) == Base.OneTo(5) (#47898)

[mbauman]

Ah, right, the generic implementation should be something that has the semantics of only(unique(map(keys, itrs...))), yeah?

[adienes]

or something like mapreduce(keys, intersect, itrs)

with the unique thing, I think a consideration would be we probably only care about equality of the elements (individual indices) rather than the type of the container returned by keys. although note that a == b does NOT necessarily imply that x[a] == x[b] even when x is indexable by both a and b.

julia> let x = fill(7, 0:4), idx = OffsetArray(rand(Bool, 5), 0:4)
           (idx == Int.(idx), x[idx] == x[Int.(idx)])
       end
(true, false)

[adienes]

breadcrumb #36175