Add docstring for keys(::AbstractArray)

[nalimilan]

This is helpful for users, and it matters because some Base functions rely on keys(::AbstractArray) returning a collection of integer ranges (e.g. hash).

Cc: @bkamins

[mbauman]

some Base functions rely on keys(::AbstractArray) returning a collection of integer ranges (e.g. hash).

Are you thinking of axes?

[bkamins]

If I understand what @naliman wants to point that hash(A::AbstractArray, h::UInt) actually requires keys to return numeric indices.

This distinction is relevant for arrays that allow for other indexing except numbers (like NamedArrays.jl) - and the question is if such arrays can return those "other indices" though keys. And we came to the conclusion that it is not allowed - but maybe it is not intended?

[nalimilan]

Thanks for the suggestion.That's fine with me too.

So keys is supposed to be "efficient", but maybe not as efficient as eachindex, right? That's how I understood it. In that case, keys for NamedArray and similar type should definitely return integer or cartesian indices rather than names. It's a bit weird that keys(::Dict) doesn't return an efficient index type (holding a reference to the slot), but I guess that's not the end of the world.

hash(a::AbstractArray) (currently) relies on the fact that keys(a) returns something that can be passed to LinearIndices, so it can't return e.g. (["a", "b"], ["A", "B"]) for a NamedMatrix.

[mbauman]

Fascinating. I'm not sure anyone has tried doing this yet — and we don't have a well-defined boundary for what needs to be overridden if keys(::AbstractArray) are something other than an integer or CartesianIndex (or if that's possible at all).

wants to point that hash(A::AbstractArray, h::UInt) actually requires keys to return numeric indices.

That's not quite how hash uses keys; I think it just depends on these two rough requirements:

• LinearIndices contains integers

• keys(a) are valid not just for a but also for LinearIndices(a) and CartesianIndices(a). In fact, I think all three of these should be valid keys for each other! For example, a rough relation that I have in my head is:

  a[keys(a)]==a && LinearIndices(a)[keys(a)] == LinearIndices(a) && CartesianIndices(a)[keys(a)] == CartesianIndices(a)

I'd be inclined to help make this work rather than simply documenting that it doesn't. In fact, NamedArrays might be in good stead for this usage if it provides NamedLinearIndices and NamedCartesianIndices and sneakily overrides the LinearIndices(::NamedArray) constructor to pass back its named version.

[mbauman]

At the same time, keys(::AbstractVector)::AbstractVector{<:Integer} is probably too well-entrenched to do anything about it thanks to downstream uses of find.

[mbauman]

So keys is supposed to be "efficient", but maybe not as efficient as eachindex, right?

I only put that in there to assuage the thought that it might return a dense Array of indices. Note that we're only documenting the ::AbstractArray method here; ::AbstractDict and ::Any have their own docstrings.

[bkamins]

Well - not knowing these details we already made DataFrameColumns object to be an AbstractVector and have keys other than integers, but at the same time it seems we are consistent what is required:

julia> df = DataFrame(rand(2,3))
2×3 DataFrame
│ Row │ x1       │ x2       │ x3       │
│     │ Float64  │ Float64  │ Float64  │
├─────┼──────────┼──────────┼──────────┤
│ 1   │ 0.276473 │ 0.961011 │ 0.329211 │
│ 2   │ 0.193074 │ 0.643824 │ 0.292972 │

julia> e = eachcol(df)
2×3 DataFrameColumns
│ Row │ x1       │ x2       │ x3       │
│     │ Float64  │ Float64  │ Float64  │
├─────┼──────────┼──────────┼──────────┤
│ 1   │ 0.276473 │ 0.961011 │ 0.329211 │
│ 2   │ 0.193074 │ 0.643824 │ 0.292972 │

julia> keys(e)
3-element Array{Symbol,1}:
 :x1
 :x2
 :x3

julia> LinearIndices(keys(e))
3-element LinearIndices{1,Tuple{Base.OneTo{Int64}}}:
 1
 2
 3

[mbauman]

Nice, that's great! That's precisely the sort of usage that I think is acceptable (and beyond that, it's great!) and I don't want to document it away.

[nalimilan]

@bkamins AFAICT DataFrameColumns doesn't satisfy the properties that @mbauman mentioned above. With your example, e[keys(e)], LinearIndices(e)[keys(e)] and CartesianIndices(e)[keys(e)] all fail. Even findfirst(x->false, e) fails actually. Not sure what would be the best solution... We could probably fix getindex and findfirst (via nextind), but should we really implement custom LinearIndices and CartesianIndices methods?

[bkamins]

Ah - you are right. I was looking at hash, but not deep enough. Unfortunately now:

julia> df = DataFrame(a=1,b=2)
1×2 DataFrame
│ Row │ a     │ b     │
│     │ Int64 │ Int64 │
├─────┼───────┼───────┤
│ 1   │ 1     │ 2     │

julia> hash(eachcol(df))
ERROR: ArgumentError: invalid index: :b of type Symbol

I will open an issue for this in DataFrames.jl

[nalimilan]

I just noticed that keys should probably also be documented in the Interfaces section for AbstractArray, right?

BTW, @mbauman, how do attempts to improve the handling of axis names in axes like JuliaArrays/AxisArrays.jl#81 fit in this discussion? Should axes and keys differ in that regard?

[bkamins]

I just noticed that keys should probably also be documented in the Interfaces section for AbstractArray, right?

Actually this is what I have also checked today :).

I think that the key thing that would be great to have decided is the rule for keys for "array-like" collections that allow "dual" indexing (with integers and with something else).

The issue is that - at least in DataFrames.jl - we found it more convenient for keys to return names and pairs return name => value mappings for such types. This is exactly what NamedTuple type does now, but it is inconsistent with what Base now assumes for arrays.

[bkamins]

So what is the decision here? Just to confirm, so that we can take appropriate steps in DataFrames.jl.

Is the contract that keys on AbstractVector must return a collection of integers? Thank you!

[StefanKarpinski]

@mbauman, what's your take?

[mbauman]

Good question.

Is the contract that keys on AbstractVector must return a collection of integers?

This is definitely the "safe" thing to do, and maybe it's what we need to demand. I was hoping that we weren't too far from making this possible, but I think it's too tricky to try to officially support at the moment.