allow offset ranges when comparing ranges

[matthias314]

This PR is prompted by the issue #54783. There I mentioned that indices are not taken into account when comparing ranges, while for AbstractArray in general they do matter. (I'm not the first to notice this.) I stumbled upon this while working on the small package OffsetRanges.jl, but I think this is relevant in general.

The purpose of the PR is to see if one can take indices into account when comparing ranges. I've run the ranges tests (with offset ranges added) and they worked fine. UPDATE: all tests pass

If one wants to address this issue, then the first question would be where offset ranges would live in the type system. One approach would be to allow existing range types to have elements that are not 1-based. This is currently done with Base.IdentityUnitRange, which is a subtype of AbstractUnitRange{Int}. This is also done with this PR, and it is the approach currently taken by OffsetArrays.jl. A different approach would be to copy the whole type tree. For example, there could be GeneralOrdinalRange that contains OrdinalRange (1-based) and OffsetOrdinalRange (variable first index). This way all existing methods for 1-based ranges would continue to work.

As said above, this PR is only meant to see if including indices when comparing ranges does work, and maybe to get further feedback. In one commit I delete the method ==(r::AbstractRange, s::AbstractRange) because I think it's redundant. It checks if the two ranges have equal elements, what is exactly what the general method for AbstractArray does (modulo checking that the axes are equal).

[matthias314]

So it seems that there are no fundamental problems with this PR. The only thing that "breaks" is that some tests compare ranges when they only want to compare their values. To make this easier, I have defined values for Base.IdentityUnitRange such that it returns a 1-based range, like for all other ranges. (Maybe one should do this for a suitable abstract type and then define additional methods for ranges that are already 1-based. Then it would automatically work for OffsetArrays.IdOffsetRange, too.)

My main question now is: Do you want to proceed in this direction at all?

[matthias314]

I did a force push, and this made the tests (in the checks) disappear. Now one cannot see anymore that they were all passes.

To find out if there are problems with other packages, I searched for all registered packages with repos on GitHub that use IdentityUnitRange (from Base) or IdOffsetRange (from OffsetArrays). (Nothing changes for 1-based ranges with this PR, so we need non-1-based ranges.) Here are my results:

• Packages that don't have "PkgEval: ok" on JuliaHub, don't precompile or fail their own tests on master: Augmentor, AxisIndices, AxisKeys, HalfIntegerArrays, ImageBase, ImageFiltering, ImageTransformations, Images, Interfaces, Oceananigans, QuasiArrays, Raven, StaticArrayInterface, StaticNumbers, TensorCast.
  (Strictly speaking, OffsetArrays is also in this category, see test broken with Julia master JuliaArrays/OffsetArrays.jl#353. I list it below due to its importance for this PR.)

• Packages that pass their own test both with master and with this PR: ArrayInterface, BitBasis, BlockArrays, DiskArrayEngine, GLTF, HePPCAT, InfiniteArrays, SparseArrays, Static

• Packages that pass their own test with master, but fail with this PR: FFTViews, OffsetArrays, PaddedViews. EDIT: In all three cases one only has to adapt the range comparisons in runtests.jl.

  • FFTViews: see matthias314/FFTViews.jl@eacd05f
  • OffsetArrays: see matthias314/OffsetArrays.jl@2021223
    The documentation also need to be updated, see matthias314/OffsetArrays.jl@4a7c2be
  • PaddedViews: see matthias314/PaddedViews.jl@6d71af1

• Not yet evaluated: FFTViews, GLTF

In total this looks promising. However, I'm surprised how many packages don't compile or fail their own tests on master. (For my tests I combined this PR with #54891 to get master working on my machine.)

[nsajko]

My main question now is: Do you want to proceed in this direction at all?

I'm not qualified to answer this, sorry.

To find out if there are problems with other packages, I searched for all registered packages

This effort wasn't required from you, FTR, 😅. The "needs pkgeval" label indicates that Julia's Nanosoldier infrastructure should be utilized to do what you did, but in an automated and systematic manner. At least that's what I meant when I added the label. Could you rebase the changes on current master again, then I'll schedule a Nanosoldier PkgEval? Sorry for the confusion, and for the late answer. EDIT: I'll try to fix the FillArrays bug before scheduling the PkgEval, to make the PkgEval more relevant.

Packages that pass their own test with master, but fail with this PR: FFTViews, OffsetArrays, PaddedViews. EDIT: In all three cases one only has to adapt the range comparisons in runtests.jl.

Could you make PRs for the relevant packages, where it makes sense, given that you already have the changes on your Github? If you do this, please xref this PR (like JuliaLang/julia#54825, I think) there.