Skip to content

Trying to allow Int8 as indices #371

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from
Draft

Trying to allow Int8 as indices #371

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ddea88f
check it out
simone-silvestri Mar 12, 2025
f136b40
add
simone-silvestri Mar 12, 2025
2133df4
this change
simone-silvestri Mar 12, 2025
dbdc028
continue it
simone-silvestri Mar 12, 2025
b47cdaa
A couple of tests do not pass
simone-silvestri Mar 12, 2025
0fbdc28
some changes
simone-silvestri Mar 12, 2025
9b7d56a
restore a test
simone-silvestri Mar 12, 2025
050ad77
Merge branch 'JuliaArrays:master' into master
simone-silvestri Mar 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 21 additions & 20 deletions src/OffsetArrays.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,13 +109,17 @@ julia> OffsetArray(a, OffsetArrays.Origin(0)) # set the origin to zero along eac


"""
struct OffsetArray{T,N,AA<:AbstractArray{T,N}} <: AbstractArray{T,N}
struct OffsetArray{T, N, AA<:AbstractArray{T,N}, I<:Integer} <: AbstractArray{T, N}
parent::AA
offsets::NTuple{N,Int}
@inline function OffsetArray{T, N, AA}(parent::AA, offsets::NTuple{N, Int}; checkoverflow = true) where {T, N, AA<:AbstractArray{T,N}}
offsets::NTuple{N,I}
@inline function OffsetArray{T, N, AA}(parent::AA, offsets::NTuple{N, I}; checkoverflow=true) where {T, N, AA<:AbstractArray{T,N}, I<:Integer}
# allocation of `map` on tuple is optimized away
checkoverflow && map(overflow_check, axes(parent), offsets)
new{T, N, AA}(parent, offsets)
new{T, N, AA, I}(parent, offsets)
end
#special case of a 0-dimensional array, offsets do not make sense here
@inline function OffsetArray{T, N, AA}(parent::AA, offsets::Tuple{}; checkoverflow=true) where {T, N, AA<:AbstractArray{T,0}}
new{T, N, AA, Int}(parent, offsets)
end
end

Expand All @@ -124,17 +128,17 @@ end

Type alias and convenience constructor for one-dimensional [`OffsetArray`](@ref)s.
"""
const OffsetVector{T,AA<:AbstractVector{T}} = OffsetArray{T,1,AA}
const OffsetVector{T,AA<:AbstractVector{T},I<:Integer} = OffsetArray{T,1,AA,I}

"""
OffsetMatrix(A, index1, index2)

Type alias and convenience constructor for two-dimensional [`OffsetArray`](@ref)s.
"""
const OffsetMatrix{T,AA<:AbstractMatrix{T}} = OffsetArray{T,2,AA}
const OffsetMatrix{T,AA<:AbstractMatrix{T},I<:Integer} = OffsetArray{T,2,AA,I}

# checks if the offset may be added to the range without overflowing
function overflow_check(r::AbstractUnitRange, offset::Integer)
function overflow_check(r::AbstractUnitRange, offset::Integer)
Base.hastypemax(eltype(r)) || return nothing
# This gives some performance boost https://github.com/JuliaLang/julia/issues/33273
throw_upper_overflow_error(val) = throw(OverflowError("offset should be <= $(typemax(Int) - val) corresponding to the axis $r, received an offset $offset"))
Expand Down Expand Up @@ -176,16 +180,13 @@ end
for FT in (:OffsetArray, :OffsetVector, :OffsetMatrix)
# Nested OffsetArrays may strip off the wrapper and collate the offsets
# empty tuples are handled here
@eval @inline function $FT(A::OffsetArray, offsets::Tuple{Vararg{Int}}; checkoverflow = true)
@eval @inline function $FT(A::OffsetArray, offsets::Tuple{Vararg{Integer}}; checkoverflow = true)
_checkindices(A, offsets, "offsets")
# ensure that the offsets may be added together without an overflow
checkoverflow && map(overflow_check, axes(A), offsets)
I = map(+, _offsets(A, parent(A)), offsets)
$FT(parent(A), I, checkoverflow = false)
end
@eval @inline function $FT(A::OffsetArray, offsets::Tuple{Integer,Vararg{Integer}}; kw...)
$FT(A, map(Int, offsets); kw...)
end

# In general, indices get converted to AbstractUnitRanges.
# CartesianIndices{N} get converted to N ranges
Expand Down Expand Up @@ -224,16 +225,13 @@ end
@inline OffsetArray{T,N}(M::AbstractArray{T,N}, I...; kw...) where {T,N} = OffsetArray{T,N,typeof(M)}(M, I...; kw...)

@inline OffsetArray{T,N,A}(M::AbstractArray{<:Any,N}, I...; kw...) where {T,N,A<:AbstractArray{T,N}} = OffsetArray{T,N,A}(M, I; kw...)
@inline function OffsetArray{T,N,A}(M::AbstractArray{<:Any,N}, I::NTuple{N,Int}; checkoverflow = true) where {T,N,A<:AbstractArray{T,N}}
@inline function OffsetArray{T,N,A}(M::AbstractArray{<:Any,N}, I::NTuple{N,Integer}; checkoverflow = true) where {T,N,A<:AbstractArray{T,N}}
checkoverflow && map(overflow_check, axes(M), I)
Mv = no_offset_view(M)
MvA = convert(A, Mv)::A
Iof = map(+, _offsets(M), I)
OffsetArray{T,N,A}(MvA, Iof, checkoverflow = false)
end
@inline function OffsetArray{T, N, AA}(parent::AbstractArray{<:Any,N}, offsets::NTuple{N, Integer}; kw...) where {T, N, AA<:AbstractArray{T,N}}
OffsetArray{T, N, AA}(parent, map(Int, offsets)::NTuple{N,Int}; kw...)
end
@inline function OffsetArray{T,N,A}(M::AbstractArray{<:Any,N}, I::Tuple{AbstractUnitRange,Vararg{AbstractUnitRange}}; kw...) where {T,N,A<:AbstractArray{T,N}}
_checkindices(M, I, "indices")
# Performance gain by wrapping the error in a function: see https://github.com/JuliaLang/julia/issues/37558
Expand Down Expand Up @@ -275,6 +273,7 @@ end
@inline OffsetArray{T}(init::ArrayInitializer, inds...; kw...) where {T} = OffsetArray{T}(init, inds; kw...)

Base.IndexStyle(::Type{OA}) where {OA<:OffsetArray} = IndexStyle(parenttype(OA))
parenttype(::Type{OffsetArray{T,N,AA,I}}) where {T,N,AA,I} = AA
parenttype(::Type{OffsetArray{T,N,AA}}) where {T,N,AA} = AA
parenttype(A::OffsetArray) = parenttype(typeof(A))

Expand Down Expand Up @@ -333,6 +332,7 @@ function Base.similar(::Type{T}, shape::Tuple{OffsetAxisKnownLength,Vararg{Offse
P = _similar_axes_or_length(T, new_shape, shape)
OffsetArray(P, map(_offset, axes(P), shape))
end

# Try to use the axes to generate the parent array type
# This is useful if the axes have special meanings, such as with static arrays
# This method is hit if at least one axis provided to similar(A, T, axes) is an IdOffsetRange
Expand Down Expand Up @@ -381,7 +381,7 @@ Base.reshape(A::OffsetArray, inds::Dims) = _reshape_nov(A, inds)
if VERSION < v"1.10.7"
# the specialized reshape(parent::AbstractVector, ::Tuple{Colon}) is available in Base at least on this version
Base.reshape(A::OffsetVector, ::Tuple{Colon}) = A
Base.reshape(A::OffsetArray, inds::Tuple{Vararg{Union{Int,Colon}}}) = _reshape_nov(A, inds)
Base.reshape(A::OffsetArray, inds::Tuple{Vararg{Union{Integer,Colon}}}) = _reshape_nov(A, inds)
end

# permutedims in Base does not preserve axes, and can not be fixed in a non-breaking way
Expand Down Expand Up @@ -430,13 +430,13 @@ end
# but that case is handled by getindex(::OffsetArray{<:Any,N}, ::Vararg{Colon,N})
@propagate_inbounds Base.getindex(A::OffsetArray, c::Colon) = A.parent[:]

@inline function Base.getindex(A::OffsetVector, i::Int)
@inline function Base.getindex(A::OffsetVector{T,AA,<:Integer}, i::Int) where {T,AA<:AbstractVector{T}}
@boundscheck checkbounds(A, i)
@inbounds parent(A)[parentindex(Base.axes1(A), i)]
end
@propagate_inbounds Base.getindex(A::OffsetArray, i::Int) = parent(A)[i]

@inline function Base.setindex!(A::OffsetArray{T,N}, val, I::Vararg{Int,N}) where {T,N}
@inline function Base.setindex!(A::OffsetArray{T,N,AA,<:Any}, val, I::Vararg{Int,N}) where {T,N,AA}
@boundscheck checkbounds(A, I...)
J = map(parentindex, axes(A), I)
@inbounds parent(A)[J...] = val
Expand All @@ -459,6 +459,7 @@ Base.in(x, A::OffsetArray) = in(x, parent(A))
Base.copy(A::OffsetArray) = parent_call(copy, A)

Base.strides(A::OffsetArray) = strides(parent(A))
Base.elsize(::Type{OffsetArray{T,N,A,I}}) where {T,N,A,I} = Base.elsize(A)
Base.elsize(::Type{OffsetArray{T,N,A}}) where {T,N,A} = Base.elsize(A)
Base.cconvert(P::Type{Ptr{T}}, A::OffsetArray{T}) where {T} = Base.cconvert(P, parent(A))
if VERSION < v"1.11-"
Expand Down Expand Up @@ -491,7 +492,7 @@ end
end

# An OffsetUnitRange might use the rapid getindex(::Array, ::AbstractUnitRange{Int}) for contiguous indexing
@propagate_inbounds function Base.getindex(A::Array, r::OffsetUnitRange{Int})
@propagate_inbounds function Base.getindex(A::Array, r::OffsetUnitRange{Integer})
B = A[_contiguousindexingtype(parent(r))]
OffsetArray(B, axes(r), checkoverflow = false)
end
Expand Down Expand Up @@ -530,7 +531,7 @@ end
# An OffsetUnitRange{Int} has an equivalent IdOffsetRange with the same values and axes,
# something similar also holds for OffsetUnitRange{BigInt}
# We may replace the former with the latter in an indexing operation to obtain a performance boost
@inline function Base.to_index(r::OffsetUnitRange{<:Union{Int,BigInt}})
@inline function Base.to_index(r::OffsetUnitRange{<:Union{Integer,BigInt}})
of = first(axes(r,1)) - 1
IdOffsetRange(_subtractoffset(parent(r), of), of)
end
Expand Down
10 changes: 5 additions & 5 deletions src/axes.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,20 +104,20 @@ end
_bool_check(::Type, r, offset) = nothing

# Construction/coercion from arbitrary AbstractUnitRanges
function IdOffsetRange{T,I}(r::AbstractUnitRange, offset::Integer = 0) where {T<:Integer,I<:AbstractUnitRange{T}}
function IdOffsetRange{T,I}(r::AbstractUnitRange, offset::Integer=Int8(0)) where {T<:Integer,I<:AbstractUnitRange{T}}
rc, o = offset_coerce(I, r)
return IdOffsetRange{T,I}(rc, convert(T, o+offset)::T)
end
function IdOffsetRange{T}(r::AbstractUnitRange, offset::Integer = 0) where T<:Integer
function IdOffsetRange{T}(r::AbstractUnitRange, offset::Integer=Int8(0)) where T<:Integer
rc = convert(AbstractUnitRange{T}, r)::AbstractUnitRange{T}
return IdOffsetRange{T,typeof(rc)}(rc, convert(T, offset)::T)
end
IdOffsetRange(r::AbstractUnitRange{T}, offset::Integer = 0) where T<:Integer =
IdOffsetRange(r::AbstractUnitRange{T}, offset::Integer=Int8(0)) where T<:Integer =
IdOffsetRange{T,typeof(r)}(r, convert(T, offset)::T)

# Coercion from other IdOffsetRanges
IdOffsetRange{T,I}(r::IdOffsetRange{T,I}) where {T<:Integer,I<:AbstractUnitRange{T}} = r
function IdOffsetRange{T,I}(r::IdOffsetRange, offset::Integer = 0) where {T<:Integer,I<:AbstractUnitRange{T}}
function IdOffsetRange{T,I}(r::IdOffsetRange, offset::Integer=Int8(0)) where {T<:Integer,I<:AbstractUnitRange{T}}
rc, offset_rc = offset_coerce(I, r.parent)
return IdOffsetRange{T,I}(rc, convert(T, r.offset + offset + offset_rc)::T)
end
Expand All @@ -134,7 +134,7 @@ _subtractindexoffset(values, indices, offset) = _subtractoffset(values, offset)
function IdOffsetRange(; values::AbstractUnitRange{<:Integer}, indices::AbstractUnitRange{<:Integer})
length(values) == length(indices) || throw(ArgumentError("values and indices must have the same length"))
values_nooffset = no_offset_view(values)
offset = first(indices) - 1
offset = first(indices) - Int8(1)
values_minus_offset = _subtractindexoffset(values_nooffset, indices, offset)
return IdOffsetRange(values_minus_offset, offset)
end
Expand Down
7 changes: 5 additions & 2 deletions src/utils.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
_indexoffset(r::AbstractRange) = first(r) - 1
_indexoffset(i::Integer) = 0
_indexlength(r::AbstractRange) = length(r)
_indexlength(i::Integer) = Int(i)
_indexlength(i::Integer) = i
_indexlength(i::Colon) = Colon()

# utility methods used in reshape
Expand All @@ -17,7 +17,10 @@ _toaxis(i) = i
_strip_IdOffsetRange(r::IdOffsetRange) = parent(r)
_strip_IdOffsetRange(r) = r

_offset(axparent::AbstractUnitRange, ax::AbstractUnitRange) = first(ax) - first(axparent)
_offset(axparent::AbstractUnitRange, ax::AbstractUnitRange{Int}) = first(ax) - first(axparent)

# Keep the provided integer if possible
_offset(axparent::AbstractUnitRange, ax::AbstractUnitRange{I}) where {I<:Integer} = first(ax) - convert(I, first(axparent))
_offset(axparent::AbstractUnitRange, ::Union{Integer, Colon}) = 1 - first(axparent)

_offsets(A::AbstractArray) = map(ax -> first(ax) - 1, axes(A))
Expand Down
71 changes: 58 additions & 13 deletions test/runtests.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ using OffsetArrays: IdentityUnitRange, no_offset_view, IIUR, Origin, IdOffsetRan
using StaticArrays
using Test

const SliceIntUR = Slice{<:AbstractUnitRange{<:Integer}}
if !isdefined(Main, :SliceIntUR)
const SliceIntUR = Slice{<:AbstractUnitRange{<:Integer}}
end

DocMeta.setdocmeta!(OffsetArrays, :DocTestSetup, :(using OffsetArrays); recursive=true)

Expand Down Expand Up @@ -46,7 +48,7 @@ end
@testset "Project meta quality checks" begin
Aqua.test_all(OffsetArrays, piracies=false)
if VERSION >= v"1.2"
doctest(OffsetArrays, manual = false)
doctest(OffsetArrays, manual=false)
end
end

Expand Down Expand Up @@ -465,7 +467,14 @@ Base.Int(a::WeirdInteger) = a
@test eltype(a) === Float64
@test axes(a) === axes(OffsetVector{Float64}(undef, inds...)) === axes(OffsetArray{Float64, 1}(undef, inds)) === axes(OffsetArray{Float64}(undef, inds))
@test axes(a) === (IdOffsetRange(Base.OneTo(4), 0), )
@test a.offsets === (0, )

OffsetType = eltype(a.offsets)

if OffsetType == BigInt # Note BigInt.((0, )) === BigInt.((0, )) is false
@test a.offsets == (0, )
else
@test a.offsets === map(OffsetType, (0, ))
end
@test axes(a.parent) == (Base.OneTo(4), )

a = OffsetVector{Nothing}(nothing, inds)
Expand All @@ -492,7 +501,12 @@ Base.Int(a::WeirdInteger) = a
# test offsets
a = OffsetVector{Float64}(undef, inds)
ax = (IdOffsetRange(Base.OneTo(4), -2), )
@test a.offsets === (-2, )
OffsetType = eltype(a.offsets)
if OffsetType == BigInt # Note BigInt.((-2, )) === BigInt.((-2, )) is false
@test a.offsets == (-2, )
else
@test a.offsets === map(OffsetType, (-2, ))
end
@test axes(a.parent) == (Base.OneTo(4), )
@test axes(a) === ax
a = OffsetVector{Nothing}(nothing, inds)
Expand All @@ -519,10 +533,15 @@ Base.Int(a::WeirdInteger) = a
oa2 = OffsetVector(a, inds)
oa3 = OffsetArray(a, inds...)
oa4 = OffsetArray(a, inds)
@test oa1 === oa2 === oa3 === oa4
if eltype(oa1.offsets) == BigInt # Note: BigInt(1) === BigInt(1) is false
@test oa1 == oa2 == oa3 == oa4
@test oa1.offsets == (-2, )
else
@test oa1 === oa2 === oa3 === oa4
@test oa1.offsets === (-2, )
end
@test axes(oa1) === (IdOffsetRange(Base.OneTo(4), -2), )
@test parent(oa1) === a
@test oa1.offsets === (-2, )
end

oa = OffsetArray(a, :)
Expand All @@ -537,7 +556,11 @@ Base.Int(a::WeirdInteger) = a
for inds in Any[.-oa.offsets, one_based_axes...]
ooa = OffsetArray(oa, inds)
@test typeof(parent(ooa)) <: Vector
@test ooa === OffsetArray(oa, inds...) === OffsetVector(oa, inds) === OffsetVector(oa, inds...)
if eltype(ooa.offsets) == BigInt # Note: BigInt(1) === BigInt(1) is false
@test ooa == OffsetArray(oa, inds...) == OffsetVector(oa, inds) == OffsetVector(oa, inds...)
else
@test ooa === OffsetArray(oa, inds...) === OffsetVector(oa, inds) === OffsetVector(oa, inds...)
end
@test ooa == a
@test axes(ooa) == axes(a)
@test axes(ooa) !== axes(a)
Expand Down Expand Up @@ -639,7 +662,11 @@ Base.Int(a::WeirdInteger) = a
@test eltype(a) === Float64
@test axes(a) === axes(OffsetMatrix{Float64}(undef, inds...)) === axes(OffsetArray{Float64, 2}(undef, inds)) === axes(OffsetArray{Float64, 2}(undef, inds...)) === axes(OffsetArray{Float64}(undef, inds))
@test axes(a) === ax
@test a.offsets === (0, 0)
if eltype(a.offsets) == BigInt # Note BigInt.((0, 0)) === BigInt.((0, 0)) is false
@test a.offsets == (0, 0)
else
@test a.offsets === map(eltype(a.offsets), (0, 0))
end
@test axes(a.parent) == (Base.OneTo(4), Base.OneTo(3))

a = OffsetMatrix{Nothing}(nothing, inds)
Expand Down Expand Up @@ -667,7 +694,11 @@ Base.Int(a::WeirdInteger) = a
# test offsets
a = OffsetMatrix{Float64}(undef, inds)
ax = (IdOffsetRange(Base.OneTo(4), -2), IdOffsetRange(Base.OneTo(3), -1))
@test a.offsets === (-2, -1)
if eltype(a.offsets) == BigInt # Note BigInt.((0, 0)) === BigInt.((0, 0)) is false
@test a.offsets == (-2, -1)
else
@test a.offsets === (-2, -1)
end
@test axes(a.parent) == (Base.OneTo(4), Base.OneTo(3))
@test axes(a) === ax
a = OffsetMatrix{Nothing}(nothing, inds)
Expand All @@ -694,13 +725,22 @@ Base.Int(a::WeirdInteger) = a
oa2 = OffsetMatrix(a, inds)
oa3 = OffsetArray(a, inds...)
oa4 = OffsetArray(a, inds)
@test oa1 === oa2 === oa3 === oa4
if eltype(oa1.offsets) == BigInt # Note BigInt.((0, 0)) === BigInt.((0, 0)) is false
@test oa1 == oa2 == oa3 == oa4
@test oa1.offsets == (-2, -1)
else
@test oa1 === oa2 === oa3 === oa4
@test oa1.offsets === (-2, -1)
end
@test axes(oa1) === ax
@test parent(oa1) === a
@test oa1.offsets === (-2, -1)
end
oa = OffsetArray(a, :, axes(a, 2))
@test oa === OffsetArray(a, (axes(oa, 1), :)) === OffsetArray(a, axes(a)) === OffsetMatrix(a, (axes(oa, 1), :)) === OffsetMatrix(a, axes(a))
if eltype(oa.offsets) == BigInt #
@test oa == OffsetArray(a, (:, axes(a, 2))) == OffsetArray(a, axes(a)) == OffsetMatrix(a, (IdOffsetRange(axes(a)[1], 0), axes(a, 2)))
else
@test oa === OffsetArray(a, (:, axes(a, 2))) === OffsetArray(a, axes(a)) === OffsetMatrix(a, (IdOffsetRange(axes(a)[1], 0), axes(a, 2)))
end
@test oa == a
@test axes(oa) == axes(a)
@test axes(oa) !== axes(a)
Expand All @@ -713,7 +753,11 @@ Base.Int(a::WeirdInteger) = a
oa = OffsetArray(a, -1, -2)
for inds in Any[.-oa.offsets, one_based_axes...]
ooa = OffsetArray(oa, inds)
@test ooa === OffsetArray(oa, inds...) === OffsetMatrix(oa, inds) === OffsetMatrix(oa, inds...)
if eltype(ooa.offsets) == BigInt # Note BigInt.((-1, -2)) === BigInt.((-1, -2)) is false
@test ooa == OffsetArray(oa, inds...) == OffsetMatrix(oa, inds) == OffsetMatrix(oa, inds...)
else
@test ooa === OffsetArray(oa, inds...) === OffsetMatrix(oa, inds) === OffsetMatrix(oa, inds...)
end
@test typeof(parent(ooa)) <: Matrix
@test ooa == a
@test axes(ooa) == axes(a)
Expand Down Expand Up @@ -814,6 +858,7 @@ Base.Int(a::WeirdInteger) = a
@test_throws TypeError OffsetArray{Float64,3,Matrix{Float64}}

# should throw a TypeError if the offsets can not be converted to Ints
# this test does not work anymore?
@test_throws TypeError OffsetVector{Int,Vector{Int}}(zeros(Int,2), (WeirdInteger(1),))
end

Expand Down
Loading