Allow the conversion of AbstractUnitRanges to OrdinalRanges

[jishnub]

Fixes #40035

Now

julia> A = OffsetArray(big(1):big(2), 0);

julia> CartesianIndices(A)
2-element CartesianIndices{1, Tuple{Main.OffsetArrays.IdOffsetRange{Int64, Base.OneTo{Int64}}}} with indices 1:2:
 CartesianIndex(1,)
 CartesianIndex(2,)

[johnnychen94]

Thanks for the fix.
#37829 becomes notorious now 😱

[jishnub]

This is perhaps not the right fix. The conversion of an AbstractUnitRange to an OrdinalRange is where the fix should go.

I'm not sure if the constructors

julia/base/range.jl

Lines 1133 to 1135 in b20de6a

	OrdinalRange{T1, T2}(r::AbstractUnitRange{T1}) where {T1, T2<:Integer} = r
	OrdinalRange{T1, T2}(r::UnitRange) where {T1, T2<:Integer} = UnitRange{T1}(r)
	OrdinalRange{T1, T2}(r::OneTo) where {T1, T2<:Integer} = OneTo{T1}(r)

are the best ones? Eg. I'm not sure if

julia> OrdinalRange{Int,BigInt}(1:2) |> step |> typeof
Int64

makes sense, as the step is not a BigInt. Perhaps we should only have the constructor OrdinalRange{T,T}(r::AbstractUnitRange) where T = AbstractUnitRange{T}(r). I wonder if this will introduce ambiguities?

[johnnychen94]

Adding a fundamental method like this might have unknown side effects. I don't know if it's suitable to backport this to 1.6 RC branch, but for this specific issue(#40035), we can get it fixed on the OffsetArray side anyway with an appropriate compat condition.

Edit:

This PR corrects the 1.6 newly added methods in #37829, so it makes sense to backport this to the 1.6-RC branch to minimize the breakage if we still have more rc releases.

[johnnychen94]

I think these methods are still needed:

julia> OrdinalRange{Int, Int8}(1:10)
1:10 # master
MethodError # current PR

We might need more tests to catch this.

[jishnub]

What should this produce? I don't think that the behavior on master is correct.

julia> OrdinalRange{Int, Int8}(1:10)
1:10

julia> OrdinalRange{Int, Int8}(1:10) |> step |> typeof
Int64

However the docstring of OrdinalRange states that OrdinalRange{T, S} has spacings of type S. The present behavior on master is not consistent with the docstring.

Since

julia> supertype(AbstractUnitRange)
OrdinalRange{T, T} where T

that is perhaps the conversion that makes sense.

[johnnychen94]

Okay, that makes sense. Given that the previous 4 methods are newly added in Julia 1.6 only(#37829), I believe it's totally fine to revert that with OrdinalRange{T, T}(r::AbstractUnitRange) method.

[jishnub]

Gentle bump, would be nice to have this fixed