Add an abstract type signaling e.g. OffsetArray safe to use (also add badges to packages)

[PallHaraldsson]

It seems to me my proposal for the new abstract type could work: https://discourse.julialang.org/t/offsetarrays-inbounds-and-confusion/81295/10?u=palli

abstract type AbstractArbitraryArray <: Any end

abstract type AbstractArray <: AbstractArbitraryArray end  # would in the future disallow non-1-based

const AbstractAArray = AbstractArbitraryArray  # maybe good to have, as short indicating OffsetArrays ok

I think this new type might belong at JuliaLang, but I at least want to discuss it first. Maybe it could start in the package and me later migrated?

I do not propose, yet, making AbstractArray not work for this package. For now this would be opt-in and when enough packages have been changed to use the new abstract type we can cross that bridge.

If this works, then it should go into the "best practices" #281 proposal. Also the badges proposal (see link).

[jishnub]

I'm unsure if I like this idea. For example, what should a Diagonal be, should it be necessarily 1-indexed? Theoretically, we should be able to create a diagonal offset array (https://github.com/JuliaArrays/OffsetArrays.jl/issues/112).

[johnnychen94]

Theoretically, we should be able to create a diagonal offset array

😱

[PallHaraldsson]

I'm unsure if I like this idea. For example, what should a Diagonal be, should it be necessarily 1-indexed?

I'm not sure I understand the problem.

For now I'm just trying to fix the immediate problem, not also fix for something not implemented yet.

I see Diagonal(A::AbstractMatrix) and it can also take in AbstractVector when constructing. Either way, it seems could get you Diagonal{OffsetArray} when provided with OffsetArray (in theory, up to Base), and I don't see it contradicting my proposal.

julia> supertype(Diagonal)
AbstractMatrix (alias for AbstractArray{T, 2} where T)

well implemented by Vector:

julia> Diagonal([1, 10, 100])
3×3 Diagonal{Int64, Vector{Int64}}

so I think would just need the corresponding AbstractArbitraryAVector too?

[johnnychen94]

Two facts:

• most codebase assumes f(A::AbstractArray)
• Diagonal (and other array wrappers, e.g., MappedArrays, StructArrays, etc), if adopt this type design, have to be AbstractArbitraryArray

Outcome:

The ecosystem not only blocks OffsetArray, but also a LOT of valid array types (including Diagonal) only because they are not subtyping of AbstractArray. This will be too breaking for the ecosystem.

[PallHaraldsson]

most codebase assumes f(A::AbstractArray) [..] Outcome: The ecosystem not only blocks OffsetArray

Right, most packages assume AbstractArray, but a lot of those packages are incorrect when used with OffsetArray (and not tested with), and there's no good way to see if they're even supposed to work already.

Changing them to use AbstractArbitraryArray would signal that, and I think it might be a good thing to break support for AbstractArray used with OffsetArray eventually, not proposing that right now.

How many array wrappers are there? A tiny finite amount much smaller than the number of packages in total in the ecosystem. Wouldn't we just have to update them before blocking OffsetArrays working with AbstractArray? Also when done, would not limit older versions of OffsetArray (depending on where blocking done, here, or in Julialang).

[PallHaraldsson]

Diagonal (and other array wrappers, e.g., MappedArrays, StructArrays, etc), if adopt this type design, have to be AbstractArbitraryArray

I think you misunderstand. To construct e.g. Diagonal, you want to accept AbstractArbitraryArray, the new most general type. Since AbstractArray is its subtype you can of course accept all those too.

What you get out however can't be any abstract type. It will be either a (dense, usually) 1-based or 0-based (a wrapper for 1-based) type.

I was thinking should length not be defined for AbstractArbitraryArray only for AbstractArray, that would also give you type checking ruling out errors for: for i in 1:length(A), forcing you to use eachindex.

I first meant to write above you get a dense array, then changed to "(dense, usually)", since I confirmed you can get sparse diagonal.

[johnnychen94]

Diagonal (and other array wrappers, e.g., MappedArrays, StructArrays, etc), if adopt this type design, have to be AbstractArbitraryArray I think you misunderstand.

Diagonal needs to subtype AbstractArbitratyArray

struct Diagonal{...} <: AbstractArbitratyArray{...}

otherwise Diagonal doesn't follow the contract that AbstractArray is 1-based indexing. For instance, people would assume that Diagonal(offset_array) is 1-based indexing, and the same indexing issue happens.

---

For a multi-dimensional array, for i in 1:length(A) works as linear indices, which means:

julia> using OffsetArrays

julia> xo = OffsetArray([1 2; 3 4], -1, -1)
2×2 OffsetArray(::Matrix{Int64}, 0:1, 0:1) with eltype Int64 with indices 0:1×0:1:
 1  2
 3  4

julia> for i in 1:length(xo)
           @show xo[i]
       end
xo[i] = 1
xo[i] = 3
xo[i] = 2
xo[i] = 4

[johnnychen94]

Close this since it's not a doable approach. "This-package-supports-offset-arrays" badge also sounds unnecessary to me compared to Base.require_one_based_indexing

[PallHaraldsson]

For example, what should a Diagonal be, should it be necessarily 1-indexed? Theoretically, we should be able to create a diagonal offset array

@timholy, @jishnub plausibly you want a diagonal offset array, but you can't have an arbitrary-based diagonal Vector, in general.

I'm trying to think of all the potential issues, and while it's valuable to have OffsetArrays, e.g. 0-based for matrices, for both axes (or just both 1-based), it IS possible to have one of them 0-based and the other 1-based.

Then there is no good solution available for diag(A) (note, it gives you a Vector; as opposed to Diagonal(A) which would work and giving the same type of strange array).

Is it useful to think of such a strange edge case? I think so, since it's possible, I find it would be ugly to support Diagonal, but not diag. Does the inform the decision on whether to support the former only? I think we should force people to "cast" their arrays to 1-based in both cases for consistency (i.e. the status quo, already possible).

@johnnychen94, Does it also inform the decision on this issue:

Close this since it's not a doable approach.

[johnnychen94]

FWIW, the same discussion reraised in https://discourse.julialang.org/t/does-abstractarray-itself-need-to-support-indexing-beyond-1-based/83324