Formalize interface for keyword arguments in indexing

[Tokazama]

Currently there is no standard meaning for keyword arguments when passed to getindex. Often keyword arguments are propagated optional arguments for some method along the call chain. However, the most popular use of keyword arguments for indexing (as far as I'm aware) is that implemented by NamedDims.jl where dimension names are the keywords. As discussed in this issue, this is problematic if the wrapper around a named dimension array alters the dimensions of an array (e.g., SubArray) because the indices associated with the keyword arguments passed on to subsequent calls don't necessary have the same meaning once the wrapper is removed (e.g., getindex(x; kwargs...) = getindex(parent(x); kwargs...)) .

In this PR I added a minimal interface for incorporating named dimensions in the indexing pipeline. This provides generic support for what is arguably the most widely used application of keyword arguments for array indexing. This still propagates kwargs when the array doesn't have any dimension names. Since there currently isn't generic propagation of kwargs for indexing this shouldn't break anything.

Somewhat orthogonal to the issue of formalizing the keywords component of the indexing interface, this also standardizes named dimensions. It would be really nice if users didn't have to bother with figuring out if the model they used returns a NamedDims.NamedDimsArray, DimensionalData.AbstractDimArray, or AxisArrays.AxisArray when using NamedDims.jl, DimensionalData.jl, or AxisArrays.jl when indexing.

Some of the code is from NamedDims.jl but a lot of it still needs to be benchmarked and tested. I figured I'd wait to do that until I knew if this was a desirable addition and if people agreed on the interface.

@oxinabox , you may be interested in this.

[chriselrod]

Are you waiting for a comment from oxinabox, or is this ready?

[Tokazama]

I'd like to see what @oxinabox , @nickrobinson251, or @mcabbott think so that I don't step on any toes by using some of the ideas generated in NamedDims.jl.

[Tokazama]

@timholy, you were part of the original discussion surrounding dimnames. Does this seem reasonable to you?

[timholy]

From a quick skim it seems pretty reasonable to me, but I can't pretend to have given it a careful read. Just a couple of comments.

[Tokazama]

Thanks @timholy and @mcabbott for taking time to provide feedback.

I'm not sure it's helpful to add much more complexity to dimensions or kwargs for the indexing interface here. Unless there are anymore suggestions I think this is ready.

[Tokazama]

@chriselrod did you want me to start incorporating traits from #61 into methods like _unsafe_getindex!? I figured that could turn into a rather lengthy discussion, so I was saving it for another PR. But I'm open to whatever you think is best.

[Petr-Hlavenka]

Hi @Tokazama, I'd like to ask, if the interface also allows to interface AxisArrays.jl. What is the way to allow a dispatch specialized on handling of 1D array with an axis composed of equidistantly sampled time series in unitful's "Time" units (e.g. nanoseconds or seconds) and a different dispatch if the axis is in frequency domain (Unitful's Hz) with non-equidistant increments (typically log10, or a 1, 3, 10, 30, ... or 1, 2, 4, 8, .... )? And does the subarray slicing support intervals in some way?
I'm not good in designing interfaces nor an expert on dispatch, but an example how to achieve the mentioned tasks using the proposed universal array interface would help me a lot to get confidence and migrate my code to this API.

[chriselrod]

@chriselrod did you want me to start incorporating traits from #61 into methods like _unsafe_getindex!? I figured that could turn into a rather lengthy discussion, so I was saving it for another PR. But I'm open to whatever you think is best.

It'd probably be better for a separate PR, focusing this one on keyword args. Good to merge this?

Everyone's fine with the @pure uses here?

[Tokazama]

Everyone's fine with the @pure uses here?

BTW. These were copied directly from NamedDims.jl and I think Lyndon White (oxinabox) implemented them. I trust his judgement on these sorts of things, but if it's still a concern I could work out a separate solution.

@phlavenk, this is very much suited to work with something like AxisArrays.jl. You're comment motivated me to clean up some of my code in AxisIndices.jl (which has a lot of comments and mysteriously named inner functions right now). So here are some minimal examples of what could get this working with some of the stuff you mentioned. (Disclaimer: consider this pseudo code b/c I haven't actually tested it for AxisArrays specifically)

ArrayInterface.dimnames(::Type{T}) where {T<:AxisArray} = AxisArrays.axisnames(T)
ArrayInterface.has_dimnames(::Type{T}) where {T<:AxisArray} = true
function ArrayInterface.to_index(::IndexStyle, axis::Axis, arg::Interval)
    return findall(in(arg), axis.val)
end
Base.getindex(x::AxisArray, args...) = ArrayInterface.getindex(x, args...)
Base.getindex(x::AxisArray; kwargs...) = ArrayInterface.getindex(x; kwargs...)
function ArrayInterface.unsafe_get_element(x::AxisArray, inds)
    return @inbounds(parent(x)[inds...])
end

You can take advantage of this currently with AxisIndices.NamedAxisArray though.

[chriselrod]

What's the status on this?

[Tokazama]

I made one more change to conform with changes to strides. There's a fallback for contiguous_axis if it's known there is a parent array and contiguous_axis isn't directly defined.

function contiguous_axis(::Type{T}) where {T}
    if parent_type(T) <: T
        return nothing
    else
        return contiguous_axis(parent_type(T))
    end
end

If that's fine then this should be ready.

[chriselrod]

Yeah, I think that's fine.
I'm wondering if we should assume that arrays inherit properties from parent_type to reduce the boiler plate for adding support to custom array types that sever mostly as wrappers for another array, with a few added features.

E.g., to add support for HybridArrays, all that ought to be needed is adding static size information, and automate translation of this to static strides through the dense dims trait.

Alternatively, we could add an "inherits from parent" trait to make this optional. That'd be a bit safer, and only slightly less convenient.
That'd be something for a separate PR of course.

[Tokazama]

I'm wondering if we should assume that arrays inherit properties from parent_type to reduce the boiler plate for adding support to custom array types that sever mostly as wrappers for another array, with a few added features.

It would be nice if this were more formally addressed so there isn't any ambiguity about how traits function with arrays. I'd be happy to help out with that discussion/PR.