Propagate indices in vector indexing of ranges with ranges #41284
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Julia's vector indexing (with OffsetArrays in the mix) follows the rule that
axes(r[s]) == axes(s)
. However, the variousgetindex
methods defined for ranges do not follow this rule, as the result may not be expressed as a range in general if the indices are preserved.OffsetArrays
currently carries out extensive type-piracy to "fix" this.Julia Base:
After loading
OffsetArrays
However this type-piracy of
getindex
is fragile, sinceBase
(and several packages) dispatch on the first argument ofgetindex
(ie, they definegetindex(r::AbstractRange, s)
), whileOffsetArrays
dispatches on the second argument (ie. it definesgetindex(r, s::AbstractUnitRange{<:Integer})
. This is a minefield of ambiguities, as witnessed recently in JuliaArrays/ArrayInterface.jl#170 (comment).This PR tries to get around this whole issue by changing how the indices of the indices are propagated. It introduces a function
withindices
that wraps the result of an indexing operation with the correct axes:withindices(r, axes(s))
has the values ofr
but the axes ofs
. NowOffsetArrays
only needs to piratewithindices
, since the package does not need to compute the values of the result of an indexing operation, only its indices. This implies firstly that fixes togetindex
inBase
get propagated immediately toOffsetArrays
, and secondlyOffsetArrays
does not introduce ambiguities by dispatching on the second argument.Currently
withindices
inBase
is a no-op, so it does not change any result and is not a breaking change. The definition of the function really only makes sense ifOffsetArrays
is loaded, in which case one obtains the correct indices.Some of the changes in this PR are copied from #41213 as it introduced some fixes for offset ranges. If this PR is merged then the other one might not be necessary. If the other one is merged then this can be rebased on that.
Aside from this there is also a bugfix:
on master:
After this PR: