From ddea88f1f86565367fe646992d7024acc389a65c Mon Sep 17 00:00:00 2001 From: Simone Silvestri Date: Wed, 12 Mar 2025 11:01:06 +0100 Subject: [PATCH 1/7] check it out --- src/OffsetArrays.jl | 58 +++++++++++++++++++++++---------------------- src/axes.jl | 10 ++++---- src/utils.jl | 2 +- 3 files changed, 36 insertions(+), 34 deletions(-) diff --git a/src/OffsetArrays.jl b/src/OffsetArrays.jl index e0b33819..9b72116b 100644 --- a/src/OffsetArrays.jl +++ b/src/OffsetArrays.jl @@ -109,13 +109,13 @@ 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 end @@ -124,29 +124,29 @@ 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} = 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} = 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::I) where I 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")) - throw_lower_overflow_error(val) = throw(OverflowError("offset should be >= $(typemin(Int) - val) corresponding to the axis $r, received an offset $offset")) + throw_upper_overflow_error(val) = throw(OverflowError("offset should be <= $(typemax(I) - val) corresponding to the axis $r, received an offset $offset")) + throw_lower_overflow_error(val) = throw(OverflowError("offset should be >= $(typemin(I) - val) corresponding to the axis $r, received an offset $offset")) # With ranges in the picture, first(r) might not necessarily be < last(r) # we therefore use the min and max of first(r) and last(r) to check for overflow firstlast_min, firstlast_max = minmax(first(r), last(r)) - if offset > 0 && firstlast_max > typemax(Int) - offset + if offset > 0 && firstlast_max > typemax(I) - offset throw_upper_overflow_error(firstlast_max) - elseif offset < 0 && firstlast_min < typemin(Int) - offset + elseif offset < 0 && firstlast_min < typemin(I) - offset throw_lower_overflow_error(firstlast_min) end return nothing @@ -176,7 +176,7 @@ 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) @@ -224,15 +224,15 @@ 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...) +@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, offsets; 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") @@ -275,7 +275,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}}) where {T,N,AA} = AA +parenttype(::Type{OffsetArray{T,N,AA,I}}) where {T,N,AA,I} = AA parenttype(A::OffsetArray) = parenttype(typeof(A)) Base.parent(A::OffsetArray) = A.parent @@ -327,12 +327,14 @@ function Base.similar(A::AbstractArray, ::Type{T}, shape::Tuple{OffsetAxisKnownL P = _similar_axes_or_length(A, T, new_shape, shape) return OffsetArray(P, map(_offset, axes(P), shape)) end -Base.similar(::Type{A}, sz::Tuple{Vararg{Int}}) where {A<:OffsetArray} = similar(Array{eltype(A)}, sz) +Base.similar(::Type{A}, sz::Tuple{Vararg{Integer}}) where {A<:OffsetArray} = similar(Array{eltype(A)}, sz) function Base.similar(::Type{T}, shape::Tuple{OffsetAxisKnownLength,Vararg{OffsetAxisKnownLength}}) where {T<:AbstractArray} new_shape = map(_strip_IdOffsetRange, shape) P = _similar_axes_or_length(T, new_shape, shape) OffsetArray(P, map(_offset, axes(P), shape)) end +similar(::Type{A}, sz::Tuple{Integer,Vararg{Integer}}) where {A<:OffsetArray} = similar(Array{eltype(A)}, sz) + # 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 @@ -381,7 +383,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 @@ -415,7 +417,7 @@ parentindex(r::IdOffsetRange, i) = i - r.offset @propagate_inbounds Base.getindex(A::OffsetArray{<:Any,0}) = A.parent[] -@inline function Base.getindex(A::OffsetArray{<:Any,N}, I::Vararg{Int,N}) where N +@inline function Base.getindex(A::OffsetArray{<:Any,N}, I::Vararg{Integer,N}) where N @boundscheck checkbounds(A, I...) J = map(parentindex, axes(A), I) @inbounds parent(A)[J...] @@ -430,25 +432,25 @@ 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{Integer,N}) where {T,N,AA} @boundscheck checkbounds(A, I...) J = map(parentindex, axes(A), I) @inbounds parent(A)[J...] = val A end -@inline function Base.setindex!(A::OffsetVector, val, i::Int) +@inline function Base.setindex!(A::OffsetVector, val, i::Integer) @boundscheck checkbounds(A, i) @inbounds parent(A)[parentindex(Base.axes1(A), i)] = val A end -@propagate_inbounds function Base.setindex!(A::OffsetArray, val, i::Int) +@propagate_inbounds function Base.setindex!(A::OffsetArray, val, i::Integer) parent(A)[i] = val A end @@ -491,7 +493,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 @@ -506,13 +508,13 @@ if VERSION <= v"1.7.0-DEV.1039" end # Linear Indexing of OffsetArrays with AbstractUnitRanges may use the faster contiguous indexing methods -@inline function Base.getindex(A::OffsetArray, r::AbstractUnitRange{Int}) +@inline function Base.getindex(A::OffsetArray, r::AbstractUnitRange{Integer}) @boundscheck checkbounds(A, r) # nD OffsetArrays do not have their linear indices shifted, so we may forward the indices provided to the parent @inbounds B = parent(A)[_contiguousindexingtype(r)] _indexedby(B, axes(r)) end -@inline function Base.getindex(A::OffsetVector, r::AbstractUnitRange{Int}) +@inline function Base.getindex(A::OffsetVector, r::AbstractUnitRange{Integer}) @boundscheck checkbounds(A, r) # OffsetVectors may have their linear indices shifted, so we subtract the offset from the indices provided @inbounds B = parent(A)[_subtractoffset(r, A.offsets[1])] @@ -520,7 +522,7 @@ end end # This method added mainly to index an OffsetRange with another range -@inline function Base.getindex(A::OffsetVector, r::AbstractRange{Int}) +@inline function Base.getindex(A::OffsetVector, r::AbstractRange{Integer}) @boundscheck checkbounds(A, r) @inbounds B = parent(A)[_subtractoffset(r, A.offsets[1])] _indexedby(B, axes(r)) @@ -530,7 +532,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 diff --git a/src/axes.jl b/src/axes.jl index 519f0dac..7815f5b3 100644 --- a/src/axes.jl +++ b/src/axes.jl @@ -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 @@ -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 diff --git a/src/utils.jl b/src/utils.jl index 0d232c12..eb563857 100644 --- a/src/utils.jl +++ b/src/utils.jl @@ -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 From f136b406078d329aa271eea94e86920b56f4805f Mon Sep 17 00:00:00 2001 From: Simone Silvestri Date: Wed, 12 Mar 2025 11:01:59 +0100 Subject: [PATCH 2/7] add --- src/axes.jl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/axes.jl b/src/axes.jl index 7815f5b3..e96c0dde 100644 --- a/src/axes.jl +++ b/src/axes.jl @@ -104,20 +104,20 @@ end _bool_check(::Type, r, offset) = nothing # Construction/coercion from arbitrary AbstractUnitRanges -function IdOffsetRange{T,I}(r::AbstractUnitRange, offset::Integer = Int8(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 = Int8(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 = Int8(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 = Int8(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 From 2133df47423340cfa333a0b1ec02a6fed72b90b9 Mon Sep 17 00:00:00 2001 From: Simone Silvestri Date: Wed, 12 Mar 2025 14:46:11 +0100 Subject: [PATCH 3/7] this change --- src/OffsetArrays.jl | 8 -------- test/runtests.jl | 2 +- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/src/OffsetArrays.jl b/src/OffsetArrays.jl index 9b72116b..5f9cd923 100644 --- a/src/OffsetArrays.jl +++ b/src/OffsetArrays.jl @@ -183,9 +183,6 @@ for FT in (:OffsetArray, :OffsetVector, :OffsetMatrix) 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 @@ -231,9 +228,6 @@ end 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, offsets; 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 @@ -327,13 +321,11 @@ function Base.similar(A::AbstractArray, ::Type{T}, shape::Tuple{OffsetAxisKnownL P = _similar_axes_or_length(A, T, new_shape, shape) return OffsetArray(P, map(_offset, axes(P), shape)) end -Base.similar(::Type{A}, sz::Tuple{Vararg{Integer}}) where {A<:OffsetArray} = similar(Array{eltype(A)}, sz) function Base.similar(::Type{T}, shape::Tuple{OffsetAxisKnownLength,Vararg{OffsetAxisKnownLength}}) where {T<:AbstractArray} new_shape = map(_strip_IdOffsetRange, shape) P = _similar_axes_or_length(T, new_shape, shape) OffsetArray(P, map(_offset, axes(P), shape)) end -similar(::Type{A}, sz::Tuple{Integer,Vararg{Integer}}) where {A<:OffsetArray} = similar(Array{eltype(A)}, sz) # 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 diff --git a/test/runtests.jl b/test/runtests.jl index ea599975..5df49d60 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -46,7 +46,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 From dbdc02850bfb0bd9b2df0cd975d9b97a9b5ae277 Mon Sep 17 00:00:00 2001 From: Simone Silvestri Date: Wed, 12 Mar 2025 17:15:45 +0100 Subject: [PATCH 4/7] continue it --- src/OffsetArrays.jl | 20 +++++++++++-------- src/utils.jl | 5 ++++- test/runtests.jl | 47 +++++++++++++++++++++++++++++++++------------ 3 files changed, 51 insertions(+), 21 deletions(-) diff --git a/src/OffsetArrays.jl b/src/OffsetArrays.jl index 5f9cd923..be88e210 100644 --- a/src/OffsetArrays.jl +++ b/src/OffsetArrays.jl @@ -112,11 +112,15 @@ julia> OffsetArray(a, OffsetArrays.Origin(0)) # set the origin to zero along eac struct OffsetArray{T, N, AA<:AbstractArray{T,N}, I<:Integer} <: AbstractArray{T, N} parent::AA 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} + @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, 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 """ @@ -124,29 +128,29 @@ end Type alias and convenience constructor for one-dimensional [`OffsetArray`](@ref)s. """ -const OffsetVector{T,AA<:AbstractVector{T},I} = OffsetArray{T,1,AA,I} +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},I} = OffsetArray{T,2,AA,I} +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::I) where I +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(I) - val) corresponding to the axis $r, received an offset $offset")) - throw_lower_overflow_error(val) = throw(OverflowError("offset should be >= $(typemin(I) - val) corresponding to the axis $r, received an offset $offset")) + throw_upper_overflow_error(val) = throw(OverflowError("offset should be <= $(typemax(Int) - val) corresponding to the axis $r, received an offset $offset")) + throw_lower_overflow_error(val) = throw(OverflowError("offset should be >= $(typemin(Int) - val) corresponding to the axis $r, received an offset $offset")) # With ranges in the picture, first(r) might not necessarily be < last(r) # we therefore use the min and max of first(r) and last(r) to check for overflow firstlast_min, firstlast_max = minmax(first(r), last(r)) - if offset > 0 && firstlast_max > typemax(I) - offset + if offset > 0 && firstlast_max > typemax(Int) - offset throw_upper_overflow_error(firstlast_max) - elseif offset < 0 && firstlast_min < typemin(I) - offset + elseif offset < 0 && firstlast_min < typemin(Int) - offset throw_lower_overflow_error(firstlast_min) end return nothing diff --git a/src/utils.jl b/src/utils.jl index eb563857..00b6a1dc 100644 --- a/src/utils.jl +++ b/src/utils.jl @@ -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)) diff --git a/test/runtests.jl b/test/runtests.jl index 5df49d60..bf7493b5 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -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) @@ -43,12 +45,12 @@ function same_value(r1, r2) return true end -@testset "Project meta quality checks" begin - Aqua.test_all(OffsetArrays, piracies=false) - if VERSION >= v"1.2" - doctest(OffsetArrays, manual=false) - end -end +# @testset "Project meta quality checks" begin +# Aqua.test_all(OffsetArrays, piracies=false) +# if VERSION >= v"1.2" +# doctest(OffsetArrays, manual=false) +# end +# end @testset "IdOffsetRange" begin @@ -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) @@ -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) @@ -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, :) @@ -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) From b47cdaa20daca2263b0c2ec24cc66988432c56e0 Mon Sep 17 00:00:00 2001 From: Simone Silvestri Date: Wed, 12 Mar 2025 20:04:29 +0100 Subject: [PATCH 5/7] A couple of tests do not pass --- src/OffsetArrays.jl | 9 ++++++--- test/runtests.jl | 48 +++++++++++++++++++++++++++++++++------------ 2 files changed, 41 insertions(+), 16 deletions(-) diff --git a/src/OffsetArrays.jl b/src/OffsetArrays.jl index be88e210..6a5912a2 100644 --- a/src/OffsetArrays.jl +++ b/src/OffsetArrays.jl @@ -274,6 +274,7 @@ end 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)) Base.parent(A::OffsetArray) = A.parent @@ -325,6 +326,7 @@ function Base.similar(A::AbstractArray, ::Type{T}, shape::Tuple{OffsetAxisKnownL P = _similar_axes_or_length(A, T, new_shape, shape) return OffsetArray(P, map(_offset, axes(P), shape)) end +Base.similar(::Type{A}, sz::Tuple{Vararg{Int}}) where {A<:OffsetArray} = similar(Array{eltype(A)}, sz) function Base.similar(::Type{T}, shape::Tuple{OffsetAxisKnownLength,Vararg{OffsetAxisKnownLength}}) where {T<:AbstractArray} new_shape = map(_strip_IdOffsetRange, shape) P = _similar_axes_or_length(T, new_shape, shape) @@ -457,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-" @@ -504,13 +507,13 @@ if VERSION <= v"1.7.0-DEV.1039" end # Linear Indexing of OffsetArrays with AbstractUnitRanges may use the faster contiguous indexing methods -@inline function Base.getindex(A::OffsetArray, r::AbstractUnitRange{Integer}) +@inline function Base.getindex(A::OffsetArray, r::AbstractUnitRange{Int}) @boundscheck checkbounds(A, r) # nD OffsetArrays do not have their linear indices shifted, so we may forward the indices provided to the parent @inbounds B = parent(A)[_contiguousindexingtype(r)] _indexedby(B, axes(r)) end -@inline function Base.getindex(A::OffsetVector, r::AbstractUnitRange{Integer}) +@inline function Base.getindex(A::OffsetVector, r::AbstractUnitRange{Int}) @boundscheck checkbounds(A, r) # OffsetVectors may have their linear indices shifted, so we subtract the offset from the indices provided @inbounds B = parent(A)[_subtractoffset(r, A.offsets[1])] @@ -518,7 +521,7 @@ end end # This method added mainly to index an OffsetRange with another range -@inline function Base.getindex(A::OffsetVector, r::AbstractRange{Integer}) +@inline function Base.getindex(A::OffsetVector, r::AbstractRange{Int}) @boundscheck checkbounds(A, r) @inbounds B = parent(A)[_subtractoffset(r, A.offsets[1])] _indexedby(B, axes(r)) diff --git a/test/runtests.jl b/test/runtests.jl index bf7493b5..11e46bee 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -662,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) @@ -690,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) @@ -717,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) @@ -736,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) @@ -837,7 +858,8 @@ 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 - @test_throws TypeError OffsetVector{Int,Vector{Int}}(zeros(Int,2), (WeirdInteger(1),)) + # this test does not work anymore? + # @test_throws TypeError OffsetVector{Int,Vector{Int}}(zeros(Int,2), (WeirdInteger(1),)) end @testset "custom range types" begin @@ -1873,12 +1895,12 @@ Base.reshape(M::MyBigFill, ind::Tuple{}) = MyBigFill(M.val, ind) Arsc[1,1] = 5 @test first(A) == 5 - @testset "issue #235" begin - Vec64 = zeros(6) - ind_a_64 = 3 - ind_a_32 =Int32.(ind_a_64) - @test reshape(Vec64, ind_a_32, :) == reshape(Vec64, ind_a_64, :) - end + # @testset "issue #235" begin + # Vec64 = zeros(6) + # ind_a_64 = 3 + # ind_a_32 =Int32.(ind_a_64) + # @test reshape(Vec64, ind_a_32, :) == reshape(Vec64, ind_a_64, :) + # end R = reshape(zeros(6), 2, :) @test R isa Matrix From 0fbdc284a7cdb1e2f122347c23baa02eb2c2e3de Mon Sep 17 00:00:00 2001 From: Simone Silvestri Date: Wed, 12 Mar 2025 20:14:04 +0100 Subject: [PATCH 6/7] some changes --- src/OffsetArrays.jl | 6 +++--- test/runtests.jl | 24 ++++++++++++------------ 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/OffsetArrays.jl b/src/OffsetArrays.jl index 6a5912a2..0565835b 100644 --- a/src/OffsetArrays.jl +++ b/src/OffsetArrays.jl @@ -436,19 +436,19 @@ end end @propagate_inbounds Base.getindex(A::OffsetArray, i::Int) = parent(A)[i] -@inline function Base.setindex!(A::OffsetArray{T,N,AA,<:Any}, val, I::Vararg{Integer,N}) where {T,N,AA} +@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 A end -@inline function Base.setindex!(A::OffsetVector, val, i::Integer) +@inline function Base.setindex!(A::OffsetVector, val, i::Int) @boundscheck checkbounds(A, i) @inbounds parent(A)[parentindex(Base.axes1(A), i)] = val A end -@propagate_inbounds function Base.setindex!(A::OffsetArray, val, i::Integer) +@propagate_inbounds function Base.setindex!(A::OffsetArray, val, i::Int) parent(A)[i] = val A end diff --git a/test/runtests.jl b/test/runtests.jl index 11e46bee..f5d2192a 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -45,12 +45,12 @@ function same_value(r1, r2) return true end -# @testset "Project meta quality checks" begin -# Aqua.test_all(OffsetArrays, piracies=false) -# if VERSION >= v"1.2" -# doctest(OffsetArrays, manual=false) -# end -# end +@testset "Project meta quality checks" begin + Aqua.test_all(OffsetArrays, piracies=false) + if VERSION >= v"1.2" + doctest(OffsetArrays, manual=false) + end +end @testset "IdOffsetRange" begin @@ -1895,12 +1895,12 @@ Base.reshape(M::MyBigFill, ind::Tuple{}) = MyBigFill(M.val, ind) Arsc[1,1] = 5 @test first(A) == 5 - # @testset "issue #235" begin - # Vec64 = zeros(6) - # ind_a_64 = 3 - # ind_a_32 =Int32.(ind_a_64) - # @test reshape(Vec64, ind_a_32, :) == reshape(Vec64, ind_a_64, :) - # end + @testset "issue #235" begin + Vec64 = zeros(6) + ind_a_64 = 3 + ind_a_32 =Int32.(ind_a_64) + @test reshape(Vec64, ind_a_32, :) == reshape(Vec64, ind_a_64, :) + end R = reshape(zeros(6), 2, :) @test R isa Matrix From 9b7d56a9a9461fb3cd87c5b4ab14a7868449fb4b Mon Sep 17 00:00:00 2001 From: Simone Silvestri Date: Wed, 12 Mar 2025 20:19:54 +0100 Subject: [PATCH 7/7] restore a test --- src/OffsetArrays.jl | 2 +- test/runtests.jl | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OffsetArrays.jl b/src/OffsetArrays.jl index 0565835b..74ae73f1 100644 --- a/src/OffsetArrays.jl +++ b/src/OffsetArrays.jl @@ -415,7 +415,7 @@ parentindex(r::IdOffsetRange, i) = i - r.offset @propagate_inbounds Base.getindex(A::OffsetArray{<:Any,0}) = A.parent[] -@inline function Base.getindex(A::OffsetArray{<:Any,N}, I::Vararg{Integer,N}) where N +@inline function Base.getindex(A::OffsetArray{<:Any,N}, I::Vararg{Int,N}) where N @boundscheck checkbounds(A, I...) J = map(parentindex, axes(A), I) @inbounds parent(A)[J...] diff --git a/test/runtests.jl b/test/runtests.jl index f5d2192a..1b9e4592 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -859,7 +859,7 @@ Base.Int(a::WeirdInteger) = a # 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),)) + @test_throws TypeError OffsetVector{Int,Vector{Int}}(zeros(Int,2), (WeirdInteger(1),)) end @testset "custom range types" begin