lazy parent eltype conversion during getindex

[johnnychen94]

Because the struct definition hard-coded the eltype of the parent array:

struct OffsetArray{T,N,AA<:AbstractArray{T,N}} <: AbstractArray{T,N}
    parent::AA
    offsets::NTuple{N,Int}
end

whenever the wrapper OffsetArray is created, an implicity convert(AbstractArray{T, N}, parent) will be applied:

x = Int[1, 2, 3, 4]
xo = OffsetVector{Float32}(x)
parent(xo) === x # false

@btime OffsetVector{Float32}($x); # 34.555 ns (1 allocation: 96 bytes)
@btime OffsetVector($x); # 1.660 ns (0 allocations: 0 bytes)

This is an unnecessary memory allocation to me, and I believe we could relax the type to

- struct OffsetArray{T,N,AA<:AbstractArray{T,N}} <: AbstractArray{T,N}
+ struct OffsetArray{T,N,AA<:AbstractArray} <: AbstractArray{T,N}
    parent::AA
    offsets::NTuple{N,Int}
end

and do the eltype conversion during each getindex call.

[jishnub]

This might open up a few surprises, since in my use I often use an OffsetArray and its parent interchangeably, especially for linear algebra usage

[johnnychen94]

Yes, I agree. But generally, we should move the type conversion to the caller side instead of in the callee side. https://docs.julialang.org/en/v1/manual/style-guide/#Handle-excess-argument-diversity-in-the-caller

Hence we should do

x = Float32.(get_some_vector())
xo = OffsetVector{Float32}(x)

instead of

x = get_some_vector()
xo = OffsetVector{Float32}(x)

By making the latter way lazy, we allow some room for performance tweak by saving memory usage.

We can keep this issue open until we decide to move forward to 2.0