From 67977d84e5d5a83dbf48e9d0d46c1b99fd38e35a Mon Sep 17 00:00:00 2001 From: Johnny Chen Date: Fri, 18 Sep 2020 15:37:33 +0800 Subject: [PATCH 1/9] [WIP] rework OffsetArrays constructor --- src/OffsetArrays.jl | 215 ++++++++++++++++---------------------------- src/utils.jl | 44 +++++++++ 2 files changed, 120 insertions(+), 139 deletions(-) create mode 100644 src/utils.jl diff --git a/src/OffsetArrays.jl b/src/OffsetArrays.jl index 0dcad362..d1840fdc 100644 --- a/src/OffsetArrays.jl +++ b/src/OffsetArrays.jl @@ -10,62 +10,14 @@ end export OffsetArray, OffsetMatrix, OffsetVector include("axes.jl") +include("utils.jl") -## OffsetArray -struct OffsetArray{T,N,AA<:AbstractArray} <: AbstractArray{T,N} - parent::AA - offsets::NTuple{N,Int} - function OffsetArray{T, N, AA}(parent::AA, offsets::NTuple{N, Int}) where {T, N, AA<:AbstractArray} - overflow_check.(axes(parent), offsets) - new{T, N, AA}(parent, offsets) - end -end -OffsetVector{T,AA<:AbstractArray} = OffsetArray{T,1,AA} -OffsetMatrix{T,AA<:AbstractArray} = OffsetArray{T,2,AA} - -function overflow_check(r, offset::T) where T - # This gives some performance boost https://github.com/JuliaLang/julia/issues/33273 - throw_upper_overflow_error() = throw(ArgumentError("Boundary overflow detected: offset $offset should be equal or less than $(typemax(T) - last(r))")) - throw_lower_overflow_error() = throw(ArgumentError("Boundary overflow detected: offset $offset should be equal or greater than $(typemin(T) - first(r))")) - - if offset > 0 && last(r) > typemax(T) - offset - throw_upper_overflow_error() - elseif offset < 0 && first(r) < typemin(T) - offset - throw_lower_overflow_error() - end -end - -## OffsetArray constructors - -offset(axparent::AbstractUnitRange, ax::AbstractUnitRange) = first(ax) - first(axparent) -offset(axparent::AbstractUnitRange, ax::Integer) = 1 - first(axparent) - -function OffsetArray(A::AbstractArray{T,N}, offsets::NTuple{N,Int}) where {T,N} - OffsetArray{T,N,typeof(A)}(A, offsets) -end -OffsetArray(A::AbstractArray{T,0}, offsets::Tuple{}) where T = - OffsetArray{T,0,typeof(A)}(A, ()) - -OffsetArray(A::AbstractArray{T,N}, offsets::Vararg{Int,N}) where {T,N} = - OffsetArray(A, offsets) -OffsetArray(A::AbstractArray{T,0}) where {T} = OffsetArray(A, ()) - +# Techniquely we know the length of CartesianIndices +const OffsetAxisKnownLength = Union{Integer, UnitRange, Base.OneTo, IdentityUnitRange, IdOffsetRange} +const OffsetAxis = Union{OffsetAxisKnownLength, CartesianIndices, Colon} const ArrayInitializer = Union{UndefInitializer, Missing, Nothing} -OffsetArray{T,N}(init::ArrayInitializer, inds::Indices{N}) where {T,N} = - OffsetArray(Array{T,N}(init, map(indexlength, inds)), map(indexoffset, inds)) -OffsetArray{T}(init::ArrayInitializer, inds::Indices{N}) where {T,N} = OffsetArray{T,N}(init, inds) -OffsetArray{T,N}(init::ArrayInitializer, inds::Vararg{AbstractUnitRange,N}) where {T,N} = OffsetArray{T,N}(init, inds) -OffsetArray{T}(init::ArrayInitializer, inds::Vararg{AbstractUnitRange,N}) where {T,N} = OffsetArray{T,N}(init, inds) - -# OffsetVector constructors -OffsetVector(A::AbstractVector, offset) = OffsetArray(A, offset) -OffsetVector{T}(init::ArrayInitializer, inds::AbstractUnitRange) where {T} = OffsetArray{T}(init, inds) - -# OffsetMatrix constructors -OffsetMatrix(A::AbstractMatrix, offset1, offset2) = OffsetArray(A, offset1, offset2) -OffsetMatrix(A::AbstractMatrix, I::CartesianIndices{2}) = OffsetArray(A, I) -OffsetMatrix{T}(init::ArrayInitializer, inds1::AbstractUnitRange, inds2::AbstractUnitRange) where {T} = OffsetArray{T}(init, inds1, inds2) +## OffsetArray """ OffsetArray(A, indices...) @@ -116,84 +68,81 @@ ERROR: [...] ``` """ -function OffsetArray(A::AbstractArray{T,N}, inds::NTuple{N,AbstractUnitRange}) where {T,N} - axparent = axes(A) - lA = map(length, axparent) - lI = map(length, inds) - lA == lI || throw(DimensionMismatch("supplied axes do not agree with the size of the array (got size $lA for the array and $lI for the indices")) - OffsetArray(A, map(offset, axparent, inds)) +struct OffsetArray{T,N,AA<:AbstractArray} <: AbstractArray{T,N} + parent::AA + offsets::NTuple{N,Int} + function OffsetArray{T, N, AA}(parent::AA, offsets::NTuple{N, Int}) where {T, N, AA<:AbstractArray} + overflow_check.(axes(parent), offsets) + new{T, N, AA}(parent, offsets) + end end -OffsetArray(A::AbstractArray{T,N}, inds::Vararg{AbstractUnitRange,N}) where {T,N} = - OffsetArray(A, inds) +const OffsetVector{T,AA<:AbstractArray} = OffsetArray{T,1,AA} +const OffsetMatrix{T,AA<:AbstractArray} = OffsetArray{T,2,AA} -uncolonindices(A::AbstractArray{<:Any,N}, inds::NTuple{N,Any}) where {N} = uncolonindices(axes(A), inds) -uncolonindices(ax::Tuple, inds::Tuple) = (first(inds), uncolonindices(tail(ax), tail(inds))...) -uncolonindices(ax::Tuple, inds::Tuple{Colon, Vararg{Any}}) = (first(ax), uncolonindices(tail(ax), tail(inds))...) -uncolonindices(::Tuple{}, ::Tuple{}) = () +function overflow_check(r, offset::T) where T + # This gives some performance boost https://github.com/JuliaLang/julia/issues/33273 + throw_upper_overflow_error() = throw(ArgumentError("Boundary overflow detected: offset $offset should be equal or less than $(typemax(T) - last(r))")) + throw_lower_overflow_error() = throw(ArgumentError("Boundary overflow detected: offset $offset should be equal or greater than $(typemin(T) - first(r))")) -function OffsetArray(A::AbstractArray{T,N}, inds::NTuple{N,Union{AbstractUnitRange, CartesianIndices{1}, Colon}}) where {T,N} - OffsetArray(A, uncolonindices(A, inds)) -end -OffsetArray(A::AbstractArray{T,N}, inds::Vararg{Union{AbstractUnitRange, CartesianIndices{1}, Colon},N}) where {T,N} = - OffsetArray(A, uncolonindices(A, inds)) - -# Specify offsets using CartesianIndices (issue #71) -# Support a mix of AbstractUnitRanges and CartesianIndices{1} -# Extract the range r from CartesianIndices((r,)) -function stripCartesianIndices(inds::Tuple{CartesianIndices{1},Vararg{Any}}) - I = first(inds) - Ir = convert(Tuple{AbstractUnitRange{Int}}, I) |> first - (Ir, stripCartesianIndices(tail(inds))...) -end -stripCartesianIndices(inds::Tuple)= (first(inds), stripCartesianIndices(tail(inds))...) -stripCartesianIndices(::Tuple{}) = () - -OffsetArray(A::AbstractArray{<:Any,N}, inds::NTuple{N,Union{CartesianIndices{1}, AbstractUnitRange}}) where {N} = - OffsetArray(A, stripCartesianIndices(inds)) -OffsetArray(A::AbstractArray{<:Any,N}, inds::Vararg{Union{CartesianIndices{1}, AbstractUnitRange},N}) where {N} = - OffsetArray(A, inds) - -# Support an arbitrary CartesianIndices alongside colons and ranges -# The total number of indices should equal ndims(arr) -# We split the CartesianIndices{N} into N CartesianIndices{1} indices to facilitate dispatch -splitCartesianIndices(c::CartesianIndices{0}) = () -function splitCartesianIndices(c::CartesianIndices) - c1, ct = Base.IteratorsMD.split(c, Val(1)) - (c1, splitCartesianIndices(ct)...) -end -function splitCartesianIndices(t::Tuple{CartesianIndices, Vararg{Any}}) - (splitCartesianIndices(first(t))..., splitCartesianIndices(tail(t))...) -end -function splitCartesianIndices(t::Tuple) - (first(t), splitCartesianIndices(tail(t))...) + if offset > 0 && last(r) > typemax(T) - offset + throw_upper_overflow_error() + elseif offset < 0 && first(r) < typemin(T) - offset + throw_lower_overflow_error() + end end -splitCartesianIndices(::Tuple{}) = () +## OffsetArray constructors -function OffsetArray(A::AbstractArray, inds::Tuple{Vararg{Union{AbstractUnitRange, CartesianIndices, Colon}}}) - OffsetArray(A, splitCartesianIndices(inds)) -end -function OffsetArray(A::AbstractArray, inds::Vararg{Union{AbstractUnitRange, CartesianIndices, Colon}}) - OffsetArray(A, inds) +for FT in (:OffsetArray, :OffsetVector, :OffsetMatrix) + # The only route out to inner constructor + @eval function $FT(A::AbstractArray{T}, offsets::NTuple{N, Integer}) where {T, N} + ndims(A) == N || throw(DimensionMismatch("Array dimensions should equal to number of offsets")) + OffsetArray{T, ndims(A), typeof(A)}(A, offsets) + end + # nested OffsetArrays + @eval function $FT(A::OffsetArray{T}, offsets::NTuple{N, Integer}) where {T,N} + $FT(parent(A), A.offsets .+ offsets) + end + # convert ranges to offsets + @eval function $FT(A::AbstractArray{T}, inds::NTuple{N,OffsetAxisKnownLength}) where {T,N} + axparent = axes(A) + lA = map(length, axparent) + lI = map(length, inds) + lA == lI || throw(DimensionMismatch("supplied axes do not agree with the size of the array (got size $lA for the array and $lI for the indices")) + $FT(A, map(_offset, axparent, inds)) + end + # lower CartesianIndices and Colon + @eval function $FT(A::AbstractArray{T}, inds::NTuple{N, OffsetAxis}) where {T, N} + indsN = _uncolonindices(A, _stripCartesianIndices(_splitCartesianIndices(inds))) + $FT(A, indsN) + end + @eval function $FT(A::AbstractArray{T}, inds::Vararg{OffsetAxis,N}) where {T, N} + $FT(A, _uncolonindices(A, _stripCartesianIndices(_splitCartesianIndices(inds)))) + end + @eval $FT(A::AbstractArray, inds::CartesianIndices) = $FT(A, inds.indices) end -# Add methods to initialize OffsetArrays using CartesianIndices (issue #71) -function OffsetArray{T,N}(init::ArrayInitializer, inds::Tuple{Vararg{Union{AbstractUnitRange, CartesianIndices}}}) where {T,N} - indsN = stripCartesianIndices(splitCartesianIndices(inds)) - OffsetArray{T,N}(init, indsN) +# array initialization +OffsetArray{T,N}(init::ArrayInitializer, inds::NTuple{N, OffsetAxisKnownLength}) where {T,N} = + OffsetArray(Array{T,N}(init, map(_indexlength, inds)), map(_indexoffset, inds)) +OffsetArray{T,N}(init::ArrayInitializer, inds::Vararg{OffsetAxisKnownLength,N}) where {T,N} = OffsetArray{T,N}(init, inds) +function OffsetArray{T, N}(init::ArrayInitializer, inds::NTuple{N, Union{OffsetAxisKnownLength, CartesianIndices}}) where {T, N} + OffsetArray{T, N}(init, _stripCartesianIndices(_splitCartesianIndices(inds))) end -function OffsetArray{T}(init::ArrayInitializer, inds::Tuple{Vararg{Union{AbstractUnitRange, CartesianIndices}}}) where {T} - indsN = stripCartesianIndices(splitCartesianIndices(inds)) - OffsetArray{T}(init, indsN) +function OffsetArray{T, N}(init::ArrayInitializer, inds::Vararg{Union{OffsetAxisKnownLength, CartesianIndices}}) where {T, N} + OffsetArray{T, N}(init, inds) end -OffsetArray{T,N}(init::ArrayInitializer, inds::Vararg{Union{AbstractUnitRange, CartesianIndices}}) where {T,N} = OffsetArray{T,N}(init, inds) -OffsetArray{T}(init::ArrayInitializer, inds::Vararg{Union{AbstractUnitRange, CartesianIndices}}) where {T} = OffsetArray{T}(init, inds) +OffsetArray{T, N}(init::ArrayInitializer, inds::CartesianIndices{N}) where {T,N} = OffsetArray{T, N}(init, inds.indices) -# avoid a level of indirection when nesting OffsetArrays -function OffsetArray(A::OffsetArray, offsets::NTuple{N,Int}) where {N} - OffsetArray(parent(A), offsets .+ A.offsets) +OffsetArray{T}(init::ArrayInitializer, inds::NTuple{N, OffsetAxisKnownLength}) where {T,N} = OffsetArray{T,N}(init, inds) +OffsetArray{T}(init::ArrayInitializer, inds::Vararg{OffsetAxisKnownLength,N}) where {T,N} = OffsetArray{T,N}(init, inds) +function OffsetArray{T}(init::ArrayInitializer, inds::NTuple{N, Union{OffsetAxisKnownLength, CartesianIndices}}) where {T, N} + indsN = _stripCartesianIndices(_splitCartesianIndices(inds)) # CartesianIndices might contain multiple dimensions + OffsetArray{T, length(indsN)}(init, _stripCartesianIndices(_splitCartesianIndices(inds))) +end +function OffsetArray{T}(init::ArrayInitializer, inds::Vararg{Union{OffsetAxisKnownLength, CartesianIndices}, N}) where {T, N} + OffsetArray{T}(init, inds) end -OffsetArray(A::OffsetArray{T,0}, inds::Tuple{}) where {T} = OffsetArray(parent(A), ()) -# OffsetArray(A::OffsetArray{T,N}, inds::Tuple{}) where {T,N} = error("this should never be called") +OffsetArray{T}(init::ArrayInitializer, inds::CartesianIndices{N}) where {T,N} = OffsetArray{T, N}(init, inds.indices) Base.IndexStyle(::Type{OA}) where {OA<:OffsetArray} = IndexStyle(parenttype(OA)) parenttype(::Type{OffsetArray{T,N,AA}}) where {T,N,AA} = AA @@ -211,27 +160,24 @@ Base.eachindex(::IndexLinear, A::OffsetVector) = axes(A, 1) @inline Base.axes(A::OffsetArray, d) = d <= ndims(A) ? IdOffsetRange(axes(parent(A), d), A.offsets[d]) : IdOffsetRange(axes(parent(A), d)) @inline Base.axes1(A::OffsetArray{T,0}) where {T} = IdOffsetRange(axes(parent(A), 1)) # we only need to specialize this one -const OffsetAxisKnownLength = Union{Integer, UnitRange, Base.OneTo, IdentityUnitRange, IdOffsetRange} - Base.similar(A::OffsetArray, ::Type{T}, dims::Dims) where T = similar(parent(A), T, dims) function Base.similar(A::AbstractArray, ::Type{T}, inds::Tuple{OffsetAxisKnownLength,Vararg{OffsetAxisKnownLength}}) where T - B = similar(A, T, map(indexlength, inds)) - return OffsetArray(B, map(offset, axes(B), inds)) + B = similar(A, T, map(_indexlength, inds)) + return OffsetArray(B, map(_offset, axes(B), inds)) end # reshape accepts a single colon -const OffsetAxis = Union{OffsetAxisKnownLength, Colon} Base.reshape(A::AbstractArray, inds::OffsetAxis...) = reshape(A, inds) function Base.reshape(A::AbstractArray, inds::Tuple{OffsetAxis,Vararg{OffsetAxis}}) - AR = reshape(A, map(indexlength, inds)) - return OffsetArray(AR, map(offset, axes(AR), inds)) + AR = reshape(A, map(_indexlength, inds)) + return OffsetArray(AR, map(_offset, axes(AR), inds)) end # Reshaping OffsetArrays can "pop" the original OffsetArray wrapper and return # an OffsetArray(reshape(...)) instead of an OffsetArray(reshape(OffsetArray(...))) Base.reshape(A::OffsetArray, inds::Tuple{OffsetAxis,Vararg{OffsetAxis}}) = - OffsetArray(reshape(parent(A), map(indexlength, inds)), map(indexoffset, inds)) + OffsetArray(reshape(parent(A), map(_indexlength, inds)), map(_indexoffset, inds)) # And for non-offset axes, we can just return a reshape of the parent directly Base.reshape(A::OffsetArray, inds::Tuple{Union{Integer,Base.OneTo},Vararg{Union{Integer,Base.OneTo}}}) = reshape(parent(A), inds) Base.reshape(A::OffsetArray, inds::Dims) = reshape(parent(A), inds) @@ -241,8 +187,8 @@ Base.reshape(A::OffsetArray, inds::Union{Int,Colon}...) = reshape(parent(A), ind Base.reshape(A::OffsetArray, inds::Tuple{Vararg{Union{Int,Colon}}}) = reshape(parent(A), inds) function Base.similar(::Type{T}, shape::Tuple{OffsetAxis,Vararg{OffsetAxis}}) where {T<:AbstractArray} - P = T(undef, map(indexlength, shape)) - OffsetArray(P, map(offset, axes(P), shape)) + P = T(undef, map(_indexlength, shape)) + OffsetArray(P, map(_offset, axes(P), shape)) end Base.fill(v, inds::NTuple{N, Union{Integer, AbstractUnitRange}}) where {N} = @@ -346,15 +292,6 @@ Base.pop!(A::OffsetVector) = pop!(A.parent) Base.append!(A::OffsetVector, items) = (append!(A.parent, items); A) Base.empty!(A::OffsetVector) = (empty!(A.parent); A) -### Low-level utilities ### - -indexoffset(r::AbstractRange) = first(r) - 1 -indexoffset(i::Integer) = 0 -indexoffset(i::Colon) = 0 -indexlength(r::AbstractRange) = length(r) -indexlength(i::Integer) = i -indexlength(i::Colon) = Colon() - # These functions keep the summary compact function Base.inds2string(inds::Tuple{Vararg{Union{IdOffsetRange, IdentityUnitRange{<:IdOffsetRange}}}}) Base.inds2string(map(UnitRange, inds)) diff --git a/src/utils.jl b/src/utils.jl new file mode 100644 index 00000000..0a001388 --- /dev/null +++ b/src/utils.jl @@ -0,0 +1,44 @@ +### Low-level utilities ### + +_indexoffset(r::AbstractRange) = first(r) - 1 +_indexoffset(i::Integer) = 0 +_indexoffset(i::Colon) = 0 +_indexlength(r::AbstractRange) = length(r) +_indexlength(i::Integer) = i +_indexlength(i::Colon) = Colon() + +_offset(axparent::AbstractUnitRange, ax::AbstractUnitRange) = first(ax) - first(axparent) +_offset(axparent::AbstractUnitRange, ax::CartesianIndices) = _offset(axparent, first(ax.indices)) +_offset(axparent::AbstractUnitRange, ax::Integer) = 1 - first(axparent) + +_uncolonindices(A::AbstractArray{<:Any,N}, inds::NTuple{N,Any}) where {N} = _uncolonindices(axes(A), inds) +_uncolonindices(ax::Tuple, inds::Tuple) = (first(inds), _uncolonindices(tail(ax), tail(inds))...) +_uncolonindices(ax::Tuple, inds::Tuple{Colon, Vararg{Any}}) = (first(ax), _uncolonindices(tail(ax), tail(inds))...) +_uncolonindices(::Tuple{}, ::Tuple{}) = () + +# Specify offsets using CartesianIndices (issue #71) +# Support a mix of AbstractUnitRanges and CartesianIndices{1} +# Extract the range r from CartesianIndices((r,)) +function _stripCartesianIndices(inds::Tuple{CartesianIndices{1},Vararg{Any}}) + I = first(inds) + Ir = convert(Tuple{AbstractUnitRange{Int}}, I) |> first + (Ir, _stripCartesianIndices(tail(inds))...) +end +_stripCartesianIndices(inds::Tuple)= (first(inds), _stripCartesianIndices(tail(inds))...) +_stripCartesianIndices(::Tuple{}) = () + +# Support an arbitrary CartesianIndices alongside colons and ranges +# The total number of indices should equal ndims(arr) +# We split the CartesianIndices{N} into N CartesianIndices{1} indices to facilitate dispatch +_splitCartesianIndices(c::CartesianIndices{0}) = () +function _splitCartesianIndices(c::CartesianIndices) + c1, ct = Base.IteratorsMD.split(c, Val(1)) + (c1, _splitCartesianIndices(ct)...) +end +function _splitCartesianIndices(t::Tuple{CartesianIndices, Vararg{Any}}) + (_splitCartesianIndices(first(t))..., _splitCartesianIndices(tail(t))...) +end +function _splitCartesianIndices(t::Tuple) + (first(t), _splitCartesianIndices(tail(t))...) +end +_splitCartesianIndices(::Tuple{}) = () From 07f9b3c3500e5b442bccb0d5c9b369447014f97f Mon Sep 17 00:00:00 2001 From: Johnny Chen Date: Sun, 20 Sep 2020 17:26:46 +0800 Subject: [PATCH 2/9] apply suggestion from comments --- src/OffsetArrays.jl | 34 ++++++++++++++++++---------------- src/utils.jl | 29 +++-------------------------- 2 files changed, 21 insertions(+), 42 deletions(-) diff --git a/src/OffsetArrays.jl b/src/OffsetArrays.jl index d1840fdc..2c88a18b 100644 --- a/src/OffsetArrays.jl +++ b/src/OffsetArrays.jl @@ -12,7 +12,8 @@ export OffsetArray, OffsetMatrix, OffsetVector include("axes.jl") include("utils.jl") -# Techniquely we know the length of CartesianIndices +# Technically we know the length of CartesianIndices but we need to convert it first, so here we +# don't put it in OffsetAxisKnownLength. const OffsetAxisKnownLength = Union{Integer, UnitRange, Base.OneTo, IdentityUnitRange, IdOffsetRange} const OffsetAxis = Union{OffsetAxisKnownLength, CartesianIndices, Colon} const ArrayInitializer = Union{UndefInitializer, Missing, Nothing} @@ -72,7 +73,7 @@ struct OffsetArray{T,N,AA<:AbstractArray} <: AbstractArray{T,N} parent::AA offsets::NTuple{N,Int} function OffsetArray{T, N, AA}(parent::AA, offsets::NTuple{N, Int}) where {T, N, AA<:AbstractArray} - overflow_check.(axes(parent), offsets) + @boundscheck overflow_check.(axes(parent), offsets) new{T, N, AA}(parent, offsets) end end @@ -95,13 +96,11 @@ end for FT in (:OffsetArray, :OffsetVector, :OffsetMatrix) # The only route out to inner constructor @eval function $FT(A::AbstractArray{T}, offsets::NTuple{N, Integer}) where {T, N} - ndims(A) == N || throw(DimensionMismatch("Array dimensions should equal to number of offsets")) + ndims(A) == N || throw(DimensionMismatch("The number of offsets should equal ndims(A) = $(ndims(A))")) OffsetArray{T, ndims(A), typeof(A)}(A, offsets) end # nested OffsetArrays - @eval function $FT(A::OffsetArray{T}, offsets::NTuple{N, Integer}) where {T,N} - $FT(parent(A), A.offsets .+ offsets) - end + @eval $FT(A::OffsetArray{T}, offsets::NTuple{N, Integer}) where {T,N} = $FT(parent(A), A.offsets .+ offsets) # convert ranges to offsets @eval function $FT(A::AbstractArray{T}, inds::NTuple{N,OffsetAxisKnownLength}) where {T,N} axparent = axes(A) @@ -112,13 +111,11 @@ for FT in (:OffsetArray, :OffsetVector, :OffsetMatrix) end # lower CartesianIndices and Colon @eval function $FT(A::AbstractArray{T}, inds::NTuple{N, OffsetAxis}) where {T, N} - indsN = _uncolonindices(A, _stripCartesianIndices(_splitCartesianIndices(inds))) + indsN = _uncolonindices(A, _expandCartesianIndices(inds)) $FT(A, indsN) end - @eval function $FT(A::AbstractArray{T}, inds::Vararg{OffsetAxis,N}) where {T, N} - $FT(A, _uncolonindices(A, _stripCartesianIndices(_splitCartesianIndices(inds)))) - end - @eval $FT(A::AbstractArray, inds::CartesianIndices) = $FT(A, inds.indices) + @eval $FT(A::AbstractArray{T}, inds::Vararg{OffsetAxis,N}) where {T, N} = $FT(A, inds) + @eval $FT(A::AbstractArray, inds::CartesianIndices) = $FT(A, convert(Tuple{Vararg{AbstractUnitRange{Int}}}, inds)) end # array initialization @@ -126,23 +123,28 @@ OffsetArray{T,N}(init::ArrayInitializer, inds::NTuple{N, OffsetAxisKnownLength}) OffsetArray(Array{T,N}(init, map(_indexlength, inds)), map(_indexoffset, inds)) OffsetArray{T,N}(init::ArrayInitializer, inds::Vararg{OffsetAxisKnownLength,N}) where {T,N} = OffsetArray{T,N}(init, inds) function OffsetArray{T, N}(init::ArrayInitializer, inds::NTuple{N, Union{OffsetAxisKnownLength, CartesianIndices}}) where {T, N} - OffsetArray{T, N}(init, _stripCartesianIndices(_splitCartesianIndices(inds))) + OffsetArray{T, N}(init, _expandCartesianIndices(inds)) end function OffsetArray{T, N}(init::ArrayInitializer, inds::Vararg{Union{OffsetAxisKnownLength, CartesianIndices}}) where {T, N} OffsetArray{T, N}(init, inds) end -OffsetArray{T, N}(init::ArrayInitializer, inds::CartesianIndices{N}) where {T,N} = OffsetArray{T, N}(init, inds.indices) +function OffsetArray{T, N}(init::ArrayInitializer, inds::CartesianIndices{N}) where {T,N} + OffsetArray{T, N}(init, convert(Tuple{Vararg{AbstractUnitRange{Int}}}, inds)) +end OffsetArray{T}(init::ArrayInitializer, inds::NTuple{N, OffsetAxisKnownLength}) where {T,N} = OffsetArray{T,N}(init, inds) OffsetArray{T}(init::ArrayInitializer, inds::Vararg{OffsetAxisKnownLength,N}) where {T,N} = OffsetArray{T,N}(init, inds) function OffsetArray{T}(init::ArrayInitializer, inds::NTuple{N, Union{OffsetAxisKnownLength, CartesianIndices}}) where {T, N} - indsN = _stripCartesianIndices(_splitCartesianIndices(inds)) # CartesianIndices might contain multiple dimensions - OffsetArray{T, length(indsN)}(init, _stripCartesianIndices(_splitCartesianIndices(inds))) + # N is probably not the actual dimension of the array; CartesianIndices might contain multiple dimensions + indsN = _expandCartesianIndices(inds) + OffsetArray{T, length(indsN)}(init, indsN) end function OffsetArray{T}(init::ArrayInitializer, inds::Vararg{Union{OffsetAxisKnownLength, CartesianIndices}, N}) where {T, N} OffsetArray{T}(init, inds) end -OffsetArray{T}(init::ArrayInitializer, inds::CartesianIndices{N}) where {T,N} = OffsetArray{T, N}(init, inds.indices) +function OffsetArray{T}(init::ArrayInitializer, inds::CartesianIndices{N}) where {T,N} + OffsetArray{T, N}(init, convert(Tuple{Vararg{AbstractUnitRange{Int}}}, inds)) +end Base.IndexStyle(::Type{OA}) where {OA<:OffsetArray} = IndexStyle(parenttype(OA)) parenttype(::Type{OffsetArray{T,N,AA}}) where {T,N,AA} = AA diff --git a/src/utils.jl b/src/utils.jl index 0a001388..85efe9ad 100644 --- a/src/utils.jl +++ b/src/utils.jl @@ -16,29 +16,6 @@ _uncolonindices(ax::Tuple, inds::Tuple) = (first(inds), _uncolonindices(tail(ax) _uncolonindices(ax::Tuple, inds::Tuple{Colon, Vararg{Any}}) = (first(ax), _uncolonindices(tail(ax), tail(inds))...) _uncolonindices(::Tuple{}, ::Tuple{}) = () -# Specify offsets using CartesianIndices (issue #71) -# Support a mix of AbstractUnitRanges and CartesianIndices{1} -# Extract the range r from CartesianIndices((r,)) -function _stripCartesianIndices(inds::Tuple{CartesianIndices{1},Vararg{Any}}) - I = first(inds) - Ir = convert(Tuple{AbstractUnitRange{Int}}, I) |> first - (Ir, _stripCartesianIndices(tail(inds))...) -end -_stripCartesianIndices(inds::Tuple)= (first(inds), _stripCartesianIndices(tail(inds))...) -_stripCartesianIndices(::Tuple{}) = () - -# Support an arbitrary CartesianIndices alongside colons and ranges -# The total number of indices should equal ndims(arr) -# We split the CartesianIndices{N} into N CartesianIndices{1} indices to facilitate dispatch -_splitCartesianIndices(c::CartesianIndices{0}) = () -function _splitCartesianIndices(c::CartesianIndices) - c1, ct = Base.IteratorsMD.split(c, Val(1)) - (c1, _splitCartesianIndices(ct)...) -end -function _splitCartesianIndices(t::Tuple{CartesianIndices, Vararg{Any}}) - (_splitCartesianIndices(first(t))..., _splitCartesianIndices(tail(t))...) -end -function _splitCartesianIndices(t::Tuple) - (first(t), _splitCartesianIndices(tail(t))...) -end -_splitCartesianIndices(::Tuple{}) = () +_expandCartesianIndices(inds::Tuple{<:CartesianIndices, Vararg{Any}}) = (convert(Tuple{Vararg{AbstractUnitRange{Int}}}, inds[1])..., _expandCartesianIndices(Base.tail(inds))...) +_expandCartesianIndices(inds::Tuple{Any,Vararg{Any}}) = (inds[1], _expandCartesianIndices(Base.tail(inds))...) +_expandCartesianIndices(::Tuple{}) = () From 9166dfc3f6863dfb42621c91bc3b6e042dfc0219 Mon Sep 17 00:00:00 2001 From: Johnny Chen Date: Mon, 21 Sep 2020 02:46:15 +0800 Subject: [PATCH 3/9] add more test cases for OffsetArray constructors --- src/OffsetArrays.jl | 32 ++-- test/axes.jl | 73 ++++++++ test/runtests.jl | 445 ++++++++++++++++++++++++++++---------------- 3 files changed, 371 insertions(+), 179 deletions(-) create mode 100644 test/axes.jl diff --git a/src/OffsetArrays.jl b/src/OffsetArrays.jl index 2c88a18b..ba587603 100644 --- a/src/OffsetArrays.jl +++ b/src/OffsetArrays.jl @@ -14,7 +14,8 @@ include("utils.jl") # Technically we know the length of CartesianIndices but we need to convert it first, so here we # don't put it in OffsetAxisKnownLength. -const OffsetAxisKnownLength = Union{Integer, UnitRange, Base.OneTo, IdentityUnitRange, IdOffsetRange} +# TODO: add CartesianIndices to OffsetAxisKnownLength +const OffsetAxisKnownLength = Union{Integer, AbstractUnitRange, IdOffsetRange} const OffsetAxis = Union{OffsetAxisKnownLength, CartesianIndices, Colon} const ArrayInitializer = Union{UndefInitializer, Missing, Nothing} @@ -96,7 +97,7 @@ end for FT in (:OffsetArray, :OffsetVector, :OffsetMatrix) # The only route out to inner constructor @eval function $FT(A::AbstractArray{T}, offsets::NTuple{N, Integer}) where {T, N} - ndims(A) == N || throw(DimensionMismatch("The number of offsets should equal ndims(A) = $(ndims(A))")) + ndims(A) == N || throw(DimensionMismatch("The number of offsets $(N) should equal ndims(A) = $(ndims(A))")) OffsetArray{T, ndims(A), typeof(A)}(A, offsets) end # nested OffsetArrays @@ -119,32 +120,25 @@ for FT in (:OffsetArray, :OffsetVector, :OffsetMatrix) end # array initialization -OffsetArray{T,N}(init::ArrayInitializer, inds::NTuple{N, OffsetAxisKnownLength}) where {T,N} = - OffsetArray(Array{T,N}(init, map(_indexlength, inds)), map(_indexoffset, inds)) -OffsetArray{T,N}(init::ArrayInitializer, inds::Vararg{OffsetAxisKnownLength,N}) where {T,N} = OffsetArray{T,N}(init, inds) -function OffsetArray{T, N}(init::ArrayInitializer, inds::NTuple{N, Union{OffsetAxisKnownLength, CartesianIndices}}) where {T, N} - OffsetArray{T, N}(init, _expandCartesianIndices(inds)) +function OffsetArray{T,N}(init::ArrayInitializer, inds::NTuple{N, OffsetAxisKnownLength}) where {T,N} + AA = Array{T,N}(init, map(_indexlength, inds)) + OffsetArray{T, N, typeof(AA)}(AA, map(_indexoffset, inds)) end -function OffsetArray{T, N}(init::ArrayInitializer, inds::Vararg{Union{OffsetAxisKnownLength, CartesianIndices}}) where {T, N} - OffsetArray{T, N}(init, inds) -end -function OffsetArray{T, N}(init::ArrayInitializer, inds::CartesianIndices{N}) where {T,N} - OffsetArray{T, N}(init, convert(Tuple{Vararg{AbstractUnitRange{Int}}}, inds)) +function OffsetArray{T, N}(init::ArrayInitializer, inds::NTuple{NT, Union{OffsetAxisKnownLength, CartesianIndices}}) where {T, N, NT} + # NT is probably not the actual dimension of the array; CartesianIndices might contain multiple dimensions + indsN = _expandCartesianIndices(inds) + length(indsN) == N || throw(DimensionMismatch("The number of offsets $(length(indsN)) should equal ndims(A) = $N")) + OffsetArray{T, N}(init, indsN) end +OffsetArray{T,N}(init::ArrayInitializer, inds::Union{OffsetAxisKnownLength, CartesianIndices}...) where {T,N} = OffsetArray{T,N}(init, inds) OffsetArray{T}(init::ArrayInitializer, inds::NTuple{N, OffsetAxisKnownLength}) where {T,N} = OffsetArray{T,N}(init, inds) -OffsetArray{T}(init::ArrayInitializer, inds::Vararg{OffsetAxisKnownLength,N}) where {T,N} = OffsetArray{T,N}(init, inds) function OffsetArray{T}(init::ArrayInitializer, inds::NTuple{N, Union{OffsetAxisKnownLength, CartesianIndices}}) where {T, N} # N is probably not the actual dimension of the array; CartesianIndices might contain multiple dimensions indsN = _expandCartesianIndices(inds) OffsetArray{T, length(indsN)}(init, indsN) end -function OffsetArray{T}(init::ArrayInitializer, inds::Vararg{Union{OffsetAxisKnownLength, CartesianIndices}, N}) where {T, N} - OffsetArray{T}(init, inds) -end -function OffsetArray{T}(init::ArrayInitializer, inds::CartesianIndices{N}) where {T,N} - OffsetArray{T, N}(init, convert(Tuple{Vararg{AbstractUnitRange{Int}}}, inds)) -end +OffsetArray{T}(init::ArrayInitializer, inds::Union{OffsetAxisKnownLength, CartesianIndices}...) where {T} = OffsetArray{T}(init, inds) Base.IndexStyle(::Type{OA}) where {OA<:OffsetArray} = IndexStyle(parenttype(OA)) parenttype(::Type{OffsetArray{T,N,AA}}) where {T,N,AA} = AA diff --git a/test/axes.jl b/test/axes.jl new file mode 100644 index 00000000..86b202fe --- /dev/null +++ b/test/axes.jl @@ -0,0 +1,73 @@ +@testset "IdOffsetRange" begin + function same_value(r1, r2) + length(r1) == length(r2) || return false + for (v1, v2) in zip(r1, r2) + v1 == v2 || return false + end + return true + end + function check_indexed_by(r, rindx) + for i in rindx + r[i] + end + @test_throws BoundsError r[minimum(rindx)-1] + @test_throws BoundsError r[maximum(rindx)+1] + return nothing + end + + ro = OffsetArrays.IdOffsetRange(Base.OneTo(3)) + rs = OffsetArrays.IdOffsetRange(3:5, -2) + @test typeof(ro) !== typeof(rs) + @test same_value(ro, 1:3) + check_indexed_by(ro, 1:3) + @test same_value(rs, 1:3) + check_indexed_by(rs, -1:1) + @test @inferred(typeof(ro)(ro)) === ro + @test @inferred(OffsetArrays.IdOffsetRange{Int}(ro)) === ro + @test @inferred(OffsetArrays.IdOffsetRange{Int16}(ro)) === OffsetArrays.IdOffsetRange(Base.OneTo(Int16(3))) + @test @inferred(OffsetArrays.IdOffsetRange(ro)) === ro + @test parent(ro) === ro.parent + @test parent(rs) === rs.parent + # construction/coercion preserves the values, altering the axes if needed + r2 = @inferred(typeof(rs)(ro)) + @test typeof(r2) === typeof(rs) + @test same_value(ro, 1:3) + check_indexed_by(ro, 1:3) + r2 = @inferred(typeof(ro)(rs)) + @test typeof(r2) === typeof(ro) + @test same_value(r2, 1:3) + check_indexed_by(r2, 1:3) + # check the example in the comments + r = OffsetArrays.IdOffsetRange{Int,UnitRange{Int}}(3:4) + @test same_value(r, 3:4) + check_indexed_by(r, 1:2) + r = OffsetArrays.IdOffsetRange{Int,Base.OneTo{Int}}(3:4) + @test same_value(r, 3:4) + check_indexed_by(r, 3:4) + r = OffsetArrays.IdOffsetRange{Int,Base.OneTo{Int}}(3:4, -2) + @test same_value(r, 1:2) + check_indexed_by(r, 1:2) + + # conversion preserves both the values and the axes, throwing an error if this is not possible + @test @inferred(oftype(ro, ro)) === ro + @test @inferred(convert(OffsetArrays.IdOffsetRange{Int}, ro)) === ro + @test @inferred(convert(OffsetArrays.IdOffsetRange{Int}, rs)) === rs + @test @inferred(convert(OffsetArrays.IdOffsetRange{Int16}, ro)) === OffsetArrays.IdOffsetRange(Base.OneTo(Int16(3))) + r2 = @inferred(oftype(rs, ro)) + @test typeof(r2) === typeof(rs) + @test same_value(r2, 1:3) + check_indexed_by(r2, 1:3) + # These two broken tests can be fixed by uncommenting the `convert` definitions + # in axes.jl, but unfortunately Julia may not quite be ready for this. (E.g. `reinterpretarray.jl`) + @test_broken try oftype(ro, rs); false catch err true end # replace with line below + # @test_throws ArgumentError oftype(ro, rs) + @test @inferred(oftype(ro, Base.OneTo(2))) === OffsetArrays.IdOffsetRange(Base.OneTo(2)) + @test @inferred(oftype(ro, 1:2)) === OffsetArrays.IdOffsetRange(Base.OneTo(2)) + @test_broken try oftype(ro, 3:4); false catch err true end + # @test_throws ArgumentError oftype(ro, 3:4) + + # broadcasting behavior with scalars (issue #104) + r3 = (1 .+ OffsetArrays.IdOffsetRange(3:5, -1) .+ 1) .- 1 + @test same_value(r3, 3:5) + check_indexed_by(r3, 0:2) +end diff --git a/test/runtests.jl b/test/runtests.jl index 2b573245..339d9dfb 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -6,6 +6,12 @@ using LinearAlgebra using DelimitedFiles using CatIndices: BidirectionalVector +# https://github.com/JuliaLang/julia/pull/29440 +if VERSION < v"1.1.0-DEV.389" + Base.:(:)(I::CartesianIndex{N}, J::CartesianIndex{N}) where N = + CartesianIndices(map((i,j) -> i:j, Tuple(I), Tuple(J))) +end + @testset "Project meta quality checks" begin # Not checking compat section for test-only dependencies Aqua.test_all(OffsetArrays; project_extras=true, deps_compat=true, stale_deps=true, project_toml_formatting=true) @@ -85,178 +91,271 @@ end check_indexed_by(r3, 0:2) end -@testset "Single-entry arrays in dims 0:5" begin - for n = 0:5 - for z in (OffsetArray(ones(Int,ntuple(d->1,n)), ntuple(x->x-1,n)), - fill!(OffsetArray{Float64}(undef, ntuple(x->x:x, n)), 1), - fill!(OffsetArray{Float64}(undef, ntuple(x->x:x, n)...), 1), - fill!(OffsetArray{Float64,n}(undef, ntuple(x->x:x, n)), 1), - fill!(OffsetArray{Float64,n}(undef, ntuple(x->x:x, n)...), 1)) - @test length(LinearIndices(z)) == 1 - @test axes(z) == ntuple(x->x:x, n) - @test z[1] == 1 +@testset "Constructors" begin + @testset "0-dimensional array" begin + for n = 0:5 + for z in (OffsetArray(ones(Int,ntuple(d->1,n)), ntuple(x->x-1,n)), + fill!(OffsetArray{Float64}(undef, ntuple(x->x:x, n)), 1), + fill!(OffsetArray{Float64}(undef, ntuple(x->x:x, n)...), 1), + fill!(OffsetArray{Float64,n}(undef, ntuple(x->x:x, n)), 1), + fill!(OffsetArray{Float64,n}(undef, ntuple(x->x:x, n)...), 1)) + @test length(LinearIndices(z)) == 1 + @test axes(z) == ntuple(x->x:x, n) + @test z[1] == 1 + end end + a0 = reshape([3]) + a = OffsetArray(a0) + @test axes(a) == () + @test ndims(a) == 0 + @test a[] == 3 + @test a == OffsetArray(a, ()) end - a0 = reshape([3]) - a = OffsetArray(a0) - @test axes(a) == () - @test ndims(a) == 0 - @test a[] == 3 - @test a == OffsetArray(a, ()) -end - -@testset "OffsetVector constructors" begin - local v = rand(5) - @test OffsetVector(v, -2) == OffsetArray(v, -2) - @test OffsetVector(v, -2:2) == OffsetArray(v, -2:2) - @test typeof(OffsetVector{Float64}(undef, -2:2)) == typeof(OffsetArray{Float64}(undef, -2:2)) - - # Issue #71 - ov = OffsetVector(v, -2:2) - for T in [OffsetVector, OffsetArray] - if VERSION >= v"1.1" - @test ov == T(v, CartesianIndex(-2):CartesianIndex(2)) + + @testset "OffsetVector" begin + # initialization + for inds in [(4, ), (Base.OneTo(4), ), (1:4, ), (CartesianIndex(1):CartesianIndex(4), ), (IdentityUnitRange(1:4), )] + # test indices API + a = OffsetVector{Float64}(undef, inds) + @test eltype(a) === Float64 + @test axes(a) === axes(OffsetVector{Float64}(undef, inds...)) === axes(OffsetArray{Float64, 1}(undef, inds)) + @test axes(a) === (IdOffsetRange(Base.OneTo(4), 0), ) + @test a.offsets === (0, ) + @test axes(a.parent) == (Base.OneTo(4), ) + + a = OffsetVector{Nothing}(nothing, inds) + @test eltype(a) === Nothing + @test axes(a) === axes(OffsetVector{Nothing}(nothing, inds...)) === axes(OffsetArray{Nothing, 1}(nothing, inds)) + @test axes(a) === (IdOffsetRange(Base.OneTo(4), 0), ) + + a = OffsetVector{Missing}(missing, inds) + @test eltype(a) === Missing + @test axes(a) === axes(OffsetVector{Missing}(missing, inds...)) === axes(OffsetArray{Missing, 1}(missing, inds)) + @test axes(a) === (IdOffsetRange(Base.OneTo(4), 0), ) end - @test ov == T(v, CartesianIndices((-2:2,))) - end - @test OffsetVector(v, :) == OffsetArray(v, (:,)) == OffsetArray(v, :) == OffsetArray(v, axes(v)) - @test axes(OffsetVector(v, :)) == axes(v) - - w = zeros(5:6) - @test OffsetVector(w, :) == OffsetArray(w, (:,)) == OffsetArray(w, :) == OffsetArray(w, axes(w)) - @test axes(OffsetVector(w, :)) == axes(w) - - @test axes(OffsetVector(v, typemax(Int)-length(v))) == (IdOffsetRange(axes(v)[1], typemax(Int)-length(v)), ) - @test_throws ArgumentError OffsetVector(v, typemax(Int)-length(v)+1) - ao = OffsetArray(v, typemin(Int)) - @test_nowarn OffsetArray{Float64, 1, typeof(ao)}(ao, (-1, )) - @test_throws ArgumentError OffsetArray{Float64, 1, typeof(ao)}(ao, (-2, )) # inner Constructor - @test_throws ArgumentError OffsetArray(ao, (-2, )) # convinient constructor accumulate offsets -end - -@testset "OffsetMatrix constructors" begin - local v = rand(5, 3) - @test OffsetMatrix(v, -2, -1) == OffsetArray(v, -2, -1) - @test OffsetMatrix(v, -2:2, -1:1) == OffsetArray(v, -2:2, -1:1) - @test typeof(OffsetMatrix{Float64}(undef, -2:2, -1:1)) == typeof(OffsetArray{Float64}(undef, -2:2, -1:1)) - - # Issue #71 - m = OffsetMatrix(v, -2:2, -1:1) - for T in [OffsetMatrix, OffsetArray] - if VERSION >= v"1.1" - @test m == T(v, CartesianIndex(-2, -1):CartesianIndex(2, 1)) - @test m == T(v, CartesianIndex(-2):CartesianIndex(2), CartesianIndex(-1):CartesianIndex(1)) - @test m == T(v, -2:2, CartesianIndex(-1):CartesianIndex(1)) - @test m == T(v, CartesianIndex(-2):CartesianIndex(2), -1:1) + for inds in [(-1:2, ), (CartesianIndex(-1):CartesianIndex(2), ), (IdentityUnitRange(-1:2), )] + # test offsets + a = OffsetVector{Float64}(undef, inds) + ax = (IdOffsetRange(Base.OneTo(4), -2), ) + @test a.offsets === (-2, ) + @test axes(a.parent) == (Base.OneTo(4), ) + @test axes(a) === ax + a = OffsetVector{Nothing}(nothing, inds) + @test axes(a) === ax + a = OffsetVector{Missing}(missing, inds) + @test axes(a) === ax + + for (T, t) in [(Nothing, nothing), (Missing, missing)] + a = OffsetVector{Union{T, Vector{Int}}}(undef, inds) + @test !isassigned(a, -1) + @test eltype(a) === Union{T, Vector{Int}} + @test axes(a) === ax + + a = OffsetVector{Union{T, Vector{Int}}}(t, inds) + @test a[-1] === t + end + end + @test_throws Union{ArgumentError, ErrorException} OffsetVector{Float64}(undef, -2) # only positive number works + + # convenient constructors + a = rand(4) + for inds in [(-2, ), (-1:2, ), (CartesianIndex(-1):CartesianIndex(2), ), (IdentityUnitRange(-1:2), )] + oa1 = OffsetVector(a, inds...) + oa2 = OffsetVector(a, inds) + oa3 = OffsetArray(a, inds...) + oa4 = OffsetArray(a, inds) + @test oa1 === oa2 === oa3 === oa4 + @test axes(oa1) === (IdOffsetRange(Base.OneTo(4), -2), ) + @test parent(oa1) === a + @test oa1.offsets === (-2, ) end - @test m == T(v, CartesianIndices((-2:2, -1:1))) - end - @test OffsetMatrix(v, :, :) == OffsetArray(v, (:, :)) == OffsetArray(v, :, :) == OffsetArray(v, axes(v)) - @test OffsetMatrix(v, :, 2:4) == OffsetArray(v, axes(v,1), 2:4) - @test OffsetMatrix(v, 3:7, :) == OffsetArray(v, 3:7, axes(v,2)) - @test axes(OffsetArray(v, :, :)) == axes(v) - - w = zeros(3:4, 5:6) - @test OffsetMatrix(w, :, :) == OffsetArray(w, (:, :)) == OffsetArray(w, :, :) == OffsetArray(w, axes(w)) - @test OffsetMatrix(w, :, 2:3) == OffsetArray(w, axes(w,1), 2:3) - @test OffsetMatrix(w, 0:1, :) == OffsetArray(w, 0:1, axes(w,2)) - @test axes(OffsetArray(w, :, :)) == axes(w) - - @test axes(OffsetMatrix(w, :, CartesianIndices((0:1,)))) == (3:4, 0:1) - @test axes(OffsetMatrix(w, CartesianIndices((0:1,)), :)) == (0:1, 5:6) - - @test axes(OffsetMatrix(v, typemax(Int)-size(v, 1), 0)) == (IdOffsetRange(axes(v)[1], typemax(Int)-size(v, 1)), axes(v, 2)) - @test_throws ArgumentError OffsetMatrix(v, typemax(Int)-size(v,1)+1, 0) - @test_throws ArgumentError OffsetMatrix(v, 0, typemax(Int)-size(v, 2)+1) -end - -@testset "construct OffsetArray with CartesianIndices" begin - a = rand(2, 2, 2) - oa = OffsetArray(a, 0:1, 3:4, 2:3) - @test OffsetArray(a, CartesianIndices(axes(oa))) == oa - @test axes(OffsetArray(a, :, CartesianIndices((3:4, 2:3)))) == (1:2, 3:4, 2:3) - @test axes(OffsetArray(a, 10:11, CartesianIndices((3:4, 2:3)) )) == (10:11, 3:4, 2:3) - @test axes(OffsetArray(a, CartesianIndices((3:4, 2:3)), :)) == (3:4, 2:3, 1:2) - @test axes(OffsetArray(a, CartesianIndices((3:4, 2:3)), 10:11)) == (3:4, 2:3, 10:11) - @test axes(OffsetArray(a, :, :, CartesianIndices((3:4,)) )) == (1:2, 1:2, 3:4) - @test axes(OffsetArray(a, 10:11, :, CartesianIndices((3:4,)) )) == (10:11, 1:2, 3:4) - @test axes(OffsetArray(a, 10:11, 2:3, CartesianIndices((3:4,)) )) == (10:11, 2:3, 3:4) - - # ignore empty CartesianIndices - @test OffsetArray(a, CartesianIndices(()), 0:1, :, 2:3) == OffsetArray(a, 0:1, :, 2:3) - @test OffsetArray(a, 0:1, CartesianIndices(()), :, 2:3) == OffsetArray(a, 0:1, :, 2:3) - @test OffsetArray(a, 0:1, :, CartesianIndices(()), 2:3) == OffsetArray(a, 0:1, :, 2:3) - @test OffsetArray(a, 0:1, :, 2:3, CartesianIndices(())) == OffsetArray(a, 0:1, :, 2:3) -end - -@testset "undef, missing, and nothing constructors" begin - y = OffsetArray{Float32}(undef, (IdentityUnitRange(-1:1),)) - @test axes(y) == (IdentityUnitRange(-1:1),) - @test eltype(y) === Float32 - - y = OffsetArray{Float64}(undef, -1:1, -7:7, -1:2, -5:5, -1:1, -3:3, -2:2, -1:1) - @test axes(y) == (-1:1, -7:7, -1:2, -5:5, -1:1, -3:3, -2:2, -1:1) - @test eltype(y) === Float64 - - for (T, t) in ((Missing, missing), (Nothing, nothing)) - @test !isassigned(OffsetArray{Union{T,Vector{Int}}}(undef, -1:1, -1:1), -1, -1) - @test OffsetArray{Union{T,Vector{Int}}}(t, -1:1, -1:1)[-1, -1] === t - @test !isassigned(OffsetVector{Union{T,Vector{Int}}}(undef, -1:1), -1) - @test OffsetVector{Union{T,Vector{Int}}}(t, -1:1)[-1] === t + oa = OffsetArray(a, :) + @test oa === OffsetArray(a, (:, )) === OffsetArray(a, axes(a)) === OffsetVector(a, :) === OffsetVector(a, axes(a)) + @test oa == a + @test axes(oa) == axes(a) + @test axes(oa) !== axes(a) + + # nested offset array + a = rand(4) + oa = OffsetArray(a, -1) + for inds in [(1, ), (1:4, ), (CartesianIndex(1):CartesianIndex(4), ), (IdentityUnitRange(1:4), )] + ooa = OffsetArray(oa, inds) + @test typeof(parent(ooa)) <: Vector + @test ooa === OffsetArray(oa, inds...) === OffsetVector(oa, inds) === OffsetVector(oa, inds...) + @test ooa == a + @test axes(ooa) == axes(a) + @test axes(ooa) !== axes(a) + end + + # overflow bounds check + v = rand(5) + @test axes(OffsetVector(v, typemax(Int)-length(v))) == (IdOffsetRange(axes(v)[1], typemax(Int)-length(v)), ) + @test_throws ArgumentError OffsetVector(v, typemax(Int)-length(v)+1) + ao = OffsetArray(v, typemin(Int)) + @test_nowarn OffsetArray{Float64, 1, typeof(ao)}(ao, (-1, )) + @test_throws ArgumentError OffsetArray{Float64, 1, typeof(ao)}(ao, (-2, )) # inner Constructor + @test_throws ArgumentError OffsetArray(ao, (-2, )) # convinient constructor accumulate offsets end - oa = OffsetArray{Float64,2}(undef, CartesianIndices((1:2, 3:4))) - @test axes(oa) == (1:2, 3:4) - @test eltype(oa) == Float64 - oa = OffsetArray{Float64,2}(undef, 1:2, CartesianIndices((3:4,))) - @test axes(oa) == (1:2, 3:4) - @test eltype(oa) == Float64 - oa = OffsetArray{Float64,2}(undef, (1:2, CartesianIndices((3:4,)))) - @test axes(oa) == (1:2, 3:4) - @test eltype(oa) == Float64 - oa = OffsetArray{Float64}(undef, CartesianIndices((1:2, 3:4))) - @test axes(oa) == (1:2, 3:4) - @test eltype(oa) == Float64 - oa = OffsetArray{Float64}(undef, 1:2, CartesianIndices((3:4,))) - @test axes(oa) == (1:2, 3:4) - @test eltype(oa) == Float64 - oa = OffsetArray{Float64}(undef, (1:2, CartesianIndices((3:4,)))) - @test axes(oa) == (1:2, 3:4) - @test eltype(oa) == Float64 -end - -@testset "Offset range construction" begin - r = -2:5 - y = OffsetArray(r, r) - @test axes(y) == (r,) - @test step(y) == step(r) - y = OffsetArray(r, (r,)) - @test axes(y) == (r,) + @testset "OffsetMatrix" begin + # initialization + for inds in [ + (4, 3), + (Base.OneTo(4), Base.OneTo(3)), + (1:4, 1:3), + (CartesianIndex(1, 1):CartesianIndex(4, 3), ), + (CartesianIndex(1):CartesianIndex(4), CartesianIndex(1):CartesianIndex(3)), + (IdentityUnitRange(1:4), IdentityUnitRange(1:3)) + ] + # test API + a = OffsetMatrix{Float64}(undef, inds) + ax = (IdOffsetRange(Base.OneTo(4), 0), IdOffsetRange(Base.OneTo(3), 0)) + @test eltype(a) === Float64 + @test axes(a) === axes(OffsetMatrix{Float64}(undef, inds...)) === axes(OffsetArray{Float64, 2}(undef, inds)) === axes(OffsetArray{Float64, 2}(undef, inds...)) + @test axes(a) === ax + @test a.offsets === (0, 0) + @test axes(a.parent) == (Base.OneTo(4), Base.OneTo(3)) + + a = OffsetMatrix{Nothing}(nothing, inds) + @test eltype(a) === Nothing + @test axes(a) === axes(OffsetMatrix{Nothing}(nothing, inds...)) === axes(OffsetArray{Nothing, 2}(nothing, inds)) === axes(OffsetArray{Nothing, 2}(nothing, inds...)) + @test axes(a) === ax + + a = OffsetMatrix{Missing}(missing, inds) + @test eltype(a) === Missing + @test axes(a) === axes(OffsetMatrix{Missing}(missing, inds...)) === axes(OffsetArray{Missing, 2}(missing, inds)) === axes(OffsetArray{Missing, 2}(missing, inds...)) + @test axes(a) === ax + end + @test_throws Union{ArgumentError, ErrorException} OffsetMatrix{Float64}(undef, 2, -2) # only positive numbers works + + for inds in [ + (-1:2, 0:2), + (CartesianIndex(-1, 0):CartesianIndex(2, 2), ), + (-1:2, CartesianIndex(0):CartesianIndex(2)), + (CartesianIndex(-1):CartesianIndex(2), CartesianIndex(0):CartesianIndex(2)), + (IdentityUnitRange(-1:2), 0:2) + ] + # test offsets + a = OffsetMatrix{Float64}(undef, inds) + ax = (IdOffsetRange(Base.OneTo(4), -2), IdOffsetRange(Base.OneTo(3), -1)) + @test a.offsets === (-2, -1) + @test axes(a.parent) == (Base.OneTo(4), Base.OneTo(3)) + @test axes(a) === ax + a = OffsetMatrix{Nothing}(nothing, inds) + @test axes(a) === ax + a = OffsetMatrix{Missing}(missing, inds) + @test axes(a) === ax + + for (T, t) in [(Nothing, nothing), (Missing, missing)] + a = OffsetMatrix{Union{T, Vector{Int}}}(undef, inds) + @test !isassigned(a, -1, 0) + @test eltype(a) === Union{T, Vector{Int}} + @test axes(a) === ax + + a = OffsetMatrix{Union{T, Vector{Int}}}(t, inds) + @test a[-1, 0] === t + end + end - s = -2:2:4 - r = 5:8 - y = OffsetArray(s, r) - @test axes(y) == (r,) - @test step(y) == step(s) -end + # convenient constructors + a = rand(4, 3) + for inds in [ + (-1:2, 0:2), + (CartesianIndex(-1, 0):CartesianIndex(2, 2), ), + (-1:2, CartesianIndex(0):CartesianIndex(2)), + (CartesianIndex(-1):CartesianIndex(2), CartesianIndex(0):CartesianIndex(2)), + (IdentityUnitRange(-1:2), 0:2) + ] + ax = (IdOffsetRange(Base.OneTo(4), -2), IdOffsetRange(Base.OneTo(3), -1)) + oa1 = OffsetMatrix(a, inds...) + oa2 = OffsetMatrix(a, inds) + oa3 = OffsetArray(a, inds...) + oa4 = OffsetArray(a, inds) + @test oa1 === oa2 === oa3 === oa4 + @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)) + @test oa == a + @test axes(oa) == axes(a) + @test axes(oa) !== axes(a) + + oa = OffsetMatrix(a, :, 2:4) + @test oa === OffsetMatrix(a, axes(a, 1), 2:4) === OffsetMatrix(a, (axes(oa, 1), 2:4)) + + # nested offset array + a = rand(4, 3) + oa = OffsetArray(a, -1, -2) + for inds in [ + (1, 2), + (1:4, 1:3), + (CartesianIndex(1, 1):CartesianIndex(4, 3), ), + (1:4, CartesianIndex(1):CartesianIndex(3)), + (CartesianIndex(1):CartesianIndex(4), CartesianIndex(1):CartesianIndex(3)), + (IdentityUnitRange(1:4), 1:3) + ] + ooa = OffsetArray(oa, inds) + @test ooa === OffsetArray(oa, inds...) === OffsetMatrix(oa, inds) === OffsetMatrix(oa, inds...) + @test typeof(parent(ooa)) <: Matrix + @test ooa == a + @test axes(ooa) == axes(a) + @test axes(ooa) !== axes(a) + end -@testset "OffsetArray of OffsetArray construction" begin - # guarantee no unnecessary nesting of `OffsetArray`s - r = -2:5 - d = collect(r) - y = OffsetArray(d, r) + # overflow bounds check + a = rand(4, 3) + @test axes(OffsetMatrix(a, typemax(Int)-size(a, 1), 0)) == (IdOffsetRange(axes(a)[1], typemax(Int)-size(a, 1)), axes(a, 2)) + @test_throws ArgumentError OffsetMatrix(a, typemax(Int)-size(a,1)+1, 0) + @test_throws ArgumentError OffsetMatrix(a, 0, typemax(Int)-size(a, 2)+1) + end - # range constructor - y0 = OffsetArray(y, 0:7) - @test y0[0] == r[1] - @test typeof(parent(y0)) <: Array + # no need to duplicate the 2D case here, + # only add some special test cases + @testset "OffsetArray" begin + a = rand(2, 2, 2) + oa = OffsetArray(a, 0:1, 3:4, 2:3) + @test OffsetArray(a, CartesianIndices(axes(oa))) == oa + @test axes(OffsetArray(a, :, CartesianIndices((3:4, 2:3)))) == (1:2, 3:4, 2:3) + @test axes(OffsetArray(a, 10:11, CartesianIndices((3:4, 2:3)) )) == (10:11, 3:4, 2:3) + @test axes(OffsetArray(a, CartesianIndices((3:4, 2:3)), :)) == (3:4, 2:3, 1:2) + @test axes(OffsetArray(a, CartesianIndices((3:4, 2:3)), 10:11)) == (3:4, 2:3, 10:11) + @test axes(OffsetArray(a, :, :, CartesianIndices((3:4,)) )) == (1:2, 1:2, 3:4) + @test axes(OffsetArray(a, 10:11, :, CartesianIndices((3:4,)) )) == (10:11, 1:2, 3:4) + @test axes(OffsetArray(a, 10:11, 2:3, CartesianIndices((3:4,)) )) == (10:11, 2:3, 3:4) + + # ignore empty CartesianIndices + @test OffsetArray(a, CartesianIndices(()), 0:1, :, 2:3) == OffsetArray(a, 0:1, :, 2:3) + @test OffsetArray(a, 0:1, CartesianIndices(()), :, 2:3) == OffsetArray(a, 0:1, :, 2:3) + @test OffsetArray(a, 0:1, :, CartesianIndices(()), 2:3) == OffsetArray(a, 0:1, :, 2:3) + @test OffsetArray(a, 0:1, :, 2:3, CartesianIndices(())) == OffsetArray(a, 0:1, :, 2:3) + + indices = (-1:1, -7:7, -1:2, -5:5, -1:1, -3:3, -2:2, -1:1) + y = OffsetArray{Float64}(undef, indices...); + @test axes(y) === axes(OffsetArray{Float64}(undef, indices)) + @test axes(y) === axes(OffsetArray{Float64, length(indices)}(undef, indices...)) + @test axes(y) == (-1:1, -7:7, -1:2, -5:5, -1:1, -3:3, -2:2, -1:1) + @test eltype(y) === Float64 + + @test_throws DimensionMismatch OffsetArray{Float64, 2}(undef, indices) + @test_throws DimensionMismatch OffsetArray(y, indices[1:2]) + end - # offset constructor - y1 = OffsetArray(y, +2) - @test y1[0] == r[1] - @test typeof(parent(y1)) <: Array + @testset "Offset range construction" begin + r = -2:5 + for AT in [OffsetArray, OffsetVector] + y = AT(r, r) + @test axes(y) == (r,) + @test step(y) == step(r) + y = AT(r, (r,)) + @test axes(y) == (r,) + y = AT(r, CartesianIndices((r, ))) + @test axes(y) == (r, ) + end + end end @testset "Axes supplied to constructor correspond to final result" begin @@ -392,6 +491,32 @@ end @test eachindex(S) == CartesianIndices(IdentityUnitRange.((0:1,3:4))) end +@testset "IdentityUnitRange indexing" begin + # 155 + a = OffsetVector(3:4, 2:3) + ax = IdentityUnitRange(2:3) + @test a[ax[2]] == a[ax][2] + + s = -2:2:4 + r = 5:8 + y = OffsetArray(s, r) + @test axes(y) == (r,) + @test step(y) == step(s) + + a = OffsetVector(3:4, 10:11) + ax = OffsetArrays.IdOffsetRange(5:6, 5) + @test axes(a[ax]) == axes(ax) + for i in axes(ax,1) + @test a[ax[i]] == a[ax][i] + end + + ax = IdentityUnitRange(10:11) + @test axes(a[ax]) == axes(ax) + for i in axes(ax,1) + @test a[ax[i]] == a[ax][i] + end +end + @testset "view" begin A0 = [1 3; 2 4] A = OffsetArray(A0, (-1,2)) From 622c48790691c05f61c58007f43a94d70934ebd2 Mon Sep 17 00:00:00 2001 From: Johnny Chen Date: Mon, 21 Sep 2020 03:48:23 +0800 Subject: [PATCH 4/9] more simplification --- src/OffsetArrays.jl | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/OffsetArrays.jl b/src/OffsetArrays.jl index ba587603..4c9501dd 100644 --- a/src/OffsetArrays.jl +++ b/src/OffsetArrays.jl @@ -14,7 +14,6 @@ include("utils.jl") # Technically we know the length of CartesianIndices but we need to convert it first, so here we # don't put it in OffsetAxisKnownLength. -# TODO: add CartesianIndices to OffsetAxisKnownLength const OffsetAxisKnownLength = Union{Integer, AbstractUnitRange, IdOffsetRange} const OffsetAxis = Union{OffsetAxisKnownLength, CartesianIndices, Colon} const ArrayInitializer = Union{UndefInitializer, Missing, Nothing} @@ -115,8 +114,7 @@ for FT in (:OffsetArray, :OffsetVector, :OffsetMatrix) indsN = _uncolonindices(A, _expandCartesianIndices(inds)) $FT(A, indsN) end - @eval $FT(A::AbstractArray{T}, inds::Vararg{OffsetAxis,N}) where {T, N} = $FT(A, inds) - @eval $FT(A::AbstractArray, inds::CartesianIndices) = $FT(A, convert(Tuple{Vararg{AbstractUnitRange{Int}}}, inds)) + @eval $FT(A::AbstractArray{T}, inds...) where {T, N} = $FT(A, inds) end # array initialization @@ -130,7 +128,7 @@ function OffsetArray{T, N}(init::ArrayInitializer, inds::NTuple{NT, Union{Offset length(indsN) == N || throw(DimensionMismatch("The number of offsets $(length(indsN)) should equal ndims(A) = $N")) OffsetArray{T, N}(init, indsN) end -OffsetArray{T,N}(init::ArrayInitializer, inds::Union{OffsetAxisKnownLength, CartesianIndices}...) where {T,N} = OffsetArray{T,N}(init, inds) +OffsetArray{T,N}(init::ArrayInitializer, inds...) where {T,N} = OffsetArray{T,N}(init, inds) OffsetArray{T}(init::ArrayInitializer, inds::NTuple{N, OffsetAxisKnownLength}) where {T,N} = OffsetArray{T,N}(init, inds) function OffsetArray{T}(init::ArrayInitializer, inds::NTuple{N, Union{OffsetAxisKnownLength, CartesianIndices}}) where {T, N} @@ -138,7 +136,7 @@ function OffsetArray{T}(init::ArrayInitializer, inds::NTuple{N, Union{OffsetAxis indsN = _expandCartesianIndices(inds) OffsetArray{T, length(indsN)}(init, indsN) end -OffsetArray{T}(init::ArrayInitializer, inds::Union{OffsetAxisKnownLength, CartesianIndices}...) where {T} = OffsetArray{T}(init, inds) +OffsetArray{T}(init::ArrayInitializer, inds...) where {T} = OffsetArray{T}(init, inds) Base.IndexStyle(::Type{OA}) where {OA<:OffsetArray} = IndexStyle(parenttype(OA)) parenttype(::Type{OffsetArray{T,N,AA}}) where {T,N,AA} = AA From 5a804c9601fd50d21c46ba9a068c1644e1d33fb4 Mon Sep 17 00:00:00 2001 From: Johnny Chen Date: Mon, 21 Sep 2020 03:51:03 +0800 Subject: [PATCH 5/9] remove unrelated changes --- test/axes.jl | 73 ---------------------------------------------------- 1 file changed, 73 deletions(-) delete mode 100644 test/axes.jl diff --git a/test/axes.jl b/test/axes.jl deleted file mode 100644 index 86b202fe..00000000 --- a/test/axes.jl +++ /dev/null @@ -1,73 +0,0 @@ -@testset "IdOffsetRange" begin - function same_value(r1, r2) - length(r1) == length(r2) || return false - for (v1, v2) in zip(r1, r2) - v1 == v2 || return false - end - return true - end - function check_indexed_by(r, rindx) - for i in rindx - r[i] - end - @test_throws BoundsError r[minimum(rindx)-1] - @test_throws BoundsError r[maximum(rindx)+1] - return nothing - end - - ro = OffsetArrays.IdOffsetRange(Base.OneTo(3)) - rs = OffsetArrays.IdOffsetRange(3:5, -2) - @test typeof(ro) !== typeof(rs) - @test same_value(ro, 1:3) - check_indexed_by(ro, 1:3) - @test same_value(rs, 1:3) - check_indexed_by(rs, -1:1) - @test @inferred(typeof(ro)(ro)) === ro - @test @inferred(OffsetArrays.IdOffsetRange{Int}(ro)) === ro - @test @inferred(OffsetArrays.IdOffsetRange{Int16}(ro)) === OffsetArrays.IdOffsetRange(Base.OneTo(Int16(3))) - @test @inferred(OffsetArrays.IdOffsetRange(ro)) === ro - @test parent(ro) === ro.parent - @test parent(rs) === rs.parent - # construction/coercion preserves the values, altering the axes if needed - r2 = @inferred(typeof(rs)(ro)) - @test typeof(r2) === typeof(rs) - @test same_value(ro, 1:3) - check_indexed_by(ro, 1:3) - r2 = @inferred(typeof(ro)(rs)) - @test typeof(r2) === typeof(ro) - @test same_value(r2, 1:3) - check_indexed_by(r2, 1:3) - # check the example in the comments - r = OffsetArrays.IdOffsetRange{Int,UnitRange{Int}}(3:4) - @test same_value(r, 3:4) - check_indexed_by(r, 1:2) - r = OffsetArrays.IdOffsetRange{Int,Base.OneTo{Int}}(3:4) - @test same_value(r, 3:4) - check_indexed_by(r, 3:4) - r = OffsetArrays.IdOffsetRange{Int,Base.OneTo{Int}}(3:4, -2) - @test same_value(r, 1:2) - check_indexed_by(r, 1:2) - - # conversion preserves both the values and the axes, throwing an error if this is not possible - @test @inferred(oftype(ro, ro)) === ro - @test @inferred(convert(OffsetArrays.IdOffsetRange{Int}, ro)) === ro - @test @inferred(convert(OffsetArrays.IdOffsetRange{Int}, rs)) === rs - @test @inferred(convert(OffsetArrays.IdOffsetRange{Int16}, ro)) === OffsetArrays.IdOffsetRange(Base.OneTo(Int16(3))) - r2 = @inferred(oftype(rs, ro)) - @test typeof(r2) === typeof(rs) - @test same_value(r2, 1:3) - check_indexed_by(r2, 1:3) - # These two broken tests can be fixed by uncommenting the `convert` definitions - # in axes.jl, but unfortunately Julia may not quite be ready for this. (E.g. `reinterpretarray.jl`) - @test_broken try oftype(ro, rs); false catch err true end # replace with line below - # @test_throws ArgumentError oftype(ro, rs) - @test @inferred(oftype(ro, Base.OneTo(2))) === OffsetArrays.IdOffsetRange(Base.OneTo(2)) - @test @inferred(oftype(ro, 1:2)) === OffsetArrays.IdOffsetRange(Base.OneTo(2)) - @test_broken try oftype(ro, 3:4); false catch err true end - # @test_throws ArgumentError oftype(ro, 3:4) - - # broadcasting behavior with scalars (issue #104) - r3 = (1 .+ OffsetArrays.IdOffsetRange(3:5, -1) .+ 1) .- 1 - @test same_value(r3, 3:5) - check_indexed_by(r3, 0:2) -end From 88061ad9d5cf5b9156642ae2be69612fd5271d6c Mon Sep 17 00:00:00 2001 From: Johnny Chen Date: Mon, 21 Sep 2020 19:19:55 +0800 Subject: [PATCH 6/9] Revert "more simplification" This reverts commit b660d86c213213e71e948536a6435f135ff55195. --- src/OffsetArrays.jl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/OffsetArrays.jl b/src/OffsetArrays.jl index 4c9501dd..b1defc75 100644 --- a/src/OffsetArrays.jl +++ b/src/OffsetArrays.jl @@ -114,7 +114,7 @@ for FT in (:OffsetArray, :OffsetVector, :OffsetMatrix) indsN = _uncolonindices(A, _expandCartesianIndices(inds)) $FT(A, indsN) end - @eval $FT(A::AbstractArray{T}, inds...) where {T, N} = $FT(A, inds) + @eval $FT(A::AbstractArray{T}, inds::Vararg{OffsetAxis,N}) where {T, N} = $FT(A, inds) end # array initialization @@ -128,7 +128,7 @@ function OffsetArray{T, N}(init::ArrayInitializer, inds::NTuple{NT, Union{Offset length(indsN) == N || throw(DimensionMismatch("The number of offsets $(length(indsN)) should equal ndims(A) = $N")) OffsetArray{T, N}(init, indsN) end -OffsetArray{T,N}(init::ArrayInitializer, inds...) where {T,N} = OffsetArray{T,N}(init, inds) +OffsetArray{T,N}(init::ArrayInitializer, inds::Union{OffsetAxisKnownLength, CartesianIndices}...) where {T,N} = OffsetArray{T,N}(init, inds) OffsetArray{T}(init::ArrayInitializer, inds::NTuple{N, OffsetAxisKnownLength}) where {T,N} = OffsetArray{T,N}(init, inds) function OffsetArray{T}(init::ArrayInitializer, inds::NTuple{N, Union{OffsetAxisKnownLength, CartesianIndices}}) where {T, N} @@ -136,7 +136,7 @@ function OffsetArray{T}(init::ArrayInitializer, inds::NTuple{N, Union{OffsetAxis indsN = _expandCartesianIndices(inds) OffsetArray{T, length(indsN)}(init, indsN) end -OffsetArray{T}(init::ArrayInitializer, inds...) where {T} = OffsetArray{T}(init, inds) +OffsetArray{T}(init::ArrayInitializer, inds::Union{OffsetAxisKnownLength, CartesianIndices}...) where {T} = OffsetArray{T}(init, inds) Base.IndexStyle(::Type{OA}) where {OA<:OffsetArray} = IndexStyle(parenttype(OA)) parenttype(::Type{OffsetArray{T,N,AA}}) where {T,N,AA} = AA From ffebce94e25b13f33e9770d919066a3400597090 Mon Sep 17 00:00:00 2001 From: Johnny Chen Date: Wed, 23 Sep 2020 09:03:13 +0800 Subject: [PATCH 7/9] apply suggestion from comments and fix a dispatching error --- src/OffsetArrays.jl | 4 ++-- test/runtests.jl | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/OffsetArrays.jl b/src/OffsetArrays.jl index b1defc75..6a4911e8 100644 --- a/src/OffsetArrays.jl +++ b/src/OffsetArrays.jl @@ -95,12 +95,12 @@ end for FT in (:OffsetArray, :OffsetVector, :OffsetMatrix) # The only route out to inner constructor - @eval function $FT(A::AbstractArray{T}, offsets::NTuple{N, Integer}) where {T, N} + @eval function $FT(A::AbstractArray{T, N}, offsets::NTuple{N, Integer}) where {T, N} ndims(A) == N || throw(DimensionMismatch("The number of offsets $(N) should equal ndims(A) = $(ndims(A))")) OffsetArray{T, ndims(A), typeof(A)}(A, offsets) end # nested OffsetArrays - @eval $FT(A::OffsetArray{T}, offsets::NTuple{N, Integer}) where {T,N} = $FT(parent(A), A.offsets .+ offsets) + @eval $FT(A::OffsetArray{T, N}, offsets::NTuple{N, Integer}) where {T,N} = $FT(parent(A), A.offsets .+ offsets) # convert ranges to offsets @eval function $FT(A::AbstractArray{T}, inds::NTuple{N,OffsetAxisKnownLength}) where {T,N} axparent = axes(A) diff --git a/test/runtests.jl b/test/runtests.jl index 339d9dfb..31320210 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -92,7 +92,7 @@ end end @testset "Constructors" begin - @testset "0-dimensional array" begin + @testset "Single-entry arrays in dims 0:5" begin for n = 0:5 for z in (OffsetArray(ones(Int,ntuple(d->1,n)), ntuple(x->x-1,n)), fill!(OffsetArray{Float64}(undef, ntuple(x->x:x, n)), 1), @@ -109,7 +109,9 @@ end @test axes(a) == () @test ndims(a) == 0 @test a[] == 3 - @test a == OffsetArray(a, ()) + @test a === OffsetArray(a, ()) + @test_throws DimensionMismatch OffsetArray(a, 0) + @test_throws DimensionMismatch OffsetArray(a0, 0) end @testset "OffsetVector" begin From b88fb742283bbbf6725ea47a4f27f11e5e7c65a1 Mon Sep 17 00:00:00 2001 From: Johnny Chen Date: Wed, 23 Sep 2020 09:17:29 +0800 Subject: [PATCH 8/9] more test coverage --- test/runtests.jl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/runtests.jl b/test/runtests.jl index 31320210..bb7730ec 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -209,6 +209,7 @@ end (1:4, 1:3), (CartesianIndex(1, 1):CartesianIndex(4, 3), ), (CartesianIndex(1):CartesianIndex(4), CartesianIndex(1):CartesianIndex(3)), + (CartesianIndex(1):CartesianIndex(4), 1:3), (IdentityUnitRange(1:4), IdentityUnitRange(1:3)) ] # test API @@ -237,6 +238,7 @@ end (CartesianIndex(-1, 0):CartesianIndex(2, 2), ), (-1:2, CartesianIndex(0):CartesianIndex(2)), (CartesianIndex(-1):CartesianIndex(2), CartesianIndex(0):CartesianIndex(2)), + (CartesianIndex(-1):CartesianIndex(2), 0:2), (IdentityUnitRange(-1:2), 0:2) ] # test offsets From 6469a8ca09f2d5aff65a794e4abd896848863c1f Mon Sep 17 00:00:00 2001 From: Johnny Chen Date: Wed, 23 Sep 2020 11:19:18 +0800 Subject: [PATCH 9/9] more test to improve coverage --- test/runtests.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/runtests.jl b/test/runtests.jl index bb7730ec..306b091f 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -120,7 +120,7 @@ end # test indices API a = OffsetVector{Float64}(undef, inds) @test eltype(a) === Float64 - @test axes(a) === axes(OffsetVector{Float64}(undef, inds...)) === axes(OffsetArray{Float64, 1}(undef, inds)) + @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, ) @test axes(a.parent) == (Base.OneTo(4), ) @@ -216,7 +216,7 @@ end a = OffsetMatrix{Float64}(undef, inds) ax = (IdOffsetRange(Base.OneTo(4), 0), IdOffsetRange(Base.OneTo(3), 0)) @test eltype(a) === Float64 - @test axes(a) === axes(OffsetMatrix{Float64}(undef, inds...)) === axes(OffsetArray{Float64, 2}(undef, inds)) === axes(OffsetArray{Float64, 2}(undef, inds...)) + @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) @test axes(a.parent) == (Base.OneTo(4), Base.OneTo(3))