Skip to content

Commit fe74f25

Browse files
committed
apply suggestion from comments
1 parent 5d859e3 commit fe74f25

File tree

2 files changed

+21
-42
lines changed

2 files changed

+21
-42
lines changed

src/OffsetArrays.jl

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ export OffsetArray, OffsetMatrix, OffsetVector
1212
include("axes.jl")
1313
include("utils.jl")
1414

15-
# Techniquely we know the length of CartesianIndices
15+
# Technically we know the length of CartesianIndices but we need to convert it first, so here we
16+
# don't put it in OffsetAxisKnownLength.
1617
const OffsetAxisKnownLength = Union{Integer, UnitRange, Base.OneTo, IdentityUnitRange, IdOffsetRange}
1718
const OffsetAxis = Union{OffsetAxisKnownLength, CartesianIndices, Colon}
1819
const ArrayInitializer = Union{UndefInitializer, Missing, Nothing}
@@ -72,7 +73,7 @@ struct OffsetArray{T,N,AA<:AbstractArray} <: AbstractArray{T,N}
7273
parent::AA
7374
offsets::NTuple{N,Int}
7475
function OffsetArray{T, N, AA}(parent::AA, offsets::NTuple{N, Int}) where {T, N, AA<:AbstractArray}
75-
overflow_check.(axes(parent), offsets)
76+
@boundscheck overflow_check.(axes(parent), offsets)
7677
new{T, N, AA}(parent, offsets)
7778
end
7879
end
@@ -91,13 +92,11 @@ end
9192
for FT in (:OffsetArray, :OffsetVector, :OffsetMatrix)
9293
# The only route out to inner constructor
9394
@eval function $FT(A::AbstractArray{T}, offsets::NTuple{N, Integer}) where {T, N}
94-
ndims(A) == N || throw(DimensionMismatch("Array dimensions should equal to number of offsets"))
95+
ndims(A) == N || throw(DimensionMismatch("The number of offsets should equal ndims(A) = $(ndims(A))"))
9596
OffsetArray{T, ndims(A), typeof(A)}(A, offsets)
9697
end
9798
# nested OffsetArrays
98-
@eval function $FT(A::OffsetArray{T}, offsets::NTuple{N, Integer}) where {T,N}
99-
$FT(parent(A), A.offsets .+ offsets)
100-
end
99+
@eval $FT(A::OffsetArray{T}, offsets::NTuple{N, Integer}) where {T,N} = $FT(parent(A), A.offsets .+ offsets)
101100
# convert ranges to offsets
102101
@eval function $FT(A::AbstractArray{T}, inds::NTuple{N,OffsetAxisKnownLength}) where {T,N}
103102
axparent = axes(A)
@@ -108,37 +107,40 @@ for FT in (:OffsetArray, :OffsetVector, :OffsetMatrix)
108107
end
109108
# lower CartesianIndices and Colon
110109
@eval function $FT(A::AbstractArray{T}, inds::NTuple{N, OffsetAxis}) where {T, N}
111-
indsN = _uncolonindices(A, _stripCartesianIndices(_splitCartesianIndices(inds)))
110+
indsN = _uncolonindices(A, _expandCartesianIndices(inds))
112111
$FT(A, indsN)
113112
end
114-
@eval function $FT(A::AbstractArray{T}, inds::Vararg{OffsetAxis,N}) where {T, N}
115-
$FT(A, _uncolonindices(A, _stripCartesianIndices(_splitCartesianIndices(inds))))
116-
end
117-
@eval $FT(A::AbstractArray, inds::CartesianIndices) = $FT(A, inds.indices)
113+
@eval $FT(A::AbstractArray{T}, inds::Vararg{OffsetAxis,N}) where {T, N} = $FT(A, inds)
114+
@eval $FT(A::AbstractArray, inds::CartesianIndices) = $FT(A, convert(Tuple{Vararg{AbstractUnitRange{Int}}}, inds))
118115
end
119116

120117
# array initialization
121118
OffsetArray{T,N}(init::ArrayInitializer, inds::NTuple{N, OffsetAxisKnownLength}) where {T,N} =
122119
OffsetArray(Array{T,N}(init, map(_indexlength, inds)), map(_indexoffset, inds))
123120
OffsetArray{T,N}(init::ArrayInitializer, inds::Vararg{OffsetAxisKnownLength,N}) where {T,N} = OffsetArray{T,N}(init, inds)
124121
function OffsetArray{T, N}(init::ArrayInitializer, inds::NTuple{N, Union{OffsetAxisKnownLength, CartesianIndices}}) where {T, N}
125-
OffsetArray{T, N}(init, _stripCartesianIndices(_splitCartesianIndices(inds)))
122+
OffsetArray{T, N}(init, _expandCartesianIndices(inds))
126123
end
127124
function OffsetArray{T, N}(init::ArrayInitializer, inds::Vararg{Union{OffsetAxisKnownLength, CartesianIndices}}) where {T, N}
128125
OffsetArray{T, N}(init, inds)
129126
end
130-
OffsetArray{T, N}(init::ArrayInitializer, inds::CartesianIndices{N}) where {T,N} = OffsetArray{T, N}(init, inds.indices)
127+
function OffsetArray{T, N}(init::ArrayInitializer, inds::CartesianIndices{N}) where {T,N}
128+
OffsetArray{T, N}(init, convert(Tuple{Vararg{AbstractUnitRange{Int}}}, inds))
129+
end
131130

132131
OffsetArray{T}(init::ArrayInitializer, inds::NTuple{N, OffsetAxisKnownLength}) where {T,N} = OffsetArray{T,N}(init, inds)
133132
OffsetArray{T}(init::ArrayInitializer, inds::Vararg{OffsetAxisKnownLength,N}) where {T,N} = OffsetArray{T,N}(init, inds)
134133
function OffsetArray{T}(init::ArrayInitializer, inds::NTuple{N, Union{OffsetAxisKnownLength, CartesianIndices}}) where {T, N}
135-
indsN = _stripCartesianIndices(_splitCartesianIndices(inds)) # CartesianIndices might contain multiple dimensions
136-
OffsetArray{T, length(indsN)}(init, _stripCartesianIndices(_splitCartesianIndices(inds)))
134+
# N is probably not the actual dimension of the array; CartesianIndices might contain multiple dimensions
135+
indsN = _expandCartesianIndices(inds)
136+
OffsetArray{T, length(indsN)}(init, indsN)
137137
end
138138
function OffsetArray{T}(init::ArrayInitializer, inds::Vararg{Union{OffsetAxisKnownLength, CartesianIndices}, N}) where {T, N}
139139
OffsetArray{T}(init, inds)
140140
end
141-
OffsetArray{T}(init::ArrayInitializer, inds::CartesianIndices{N}) where {T,N} = OffsetArray{T, N}(init, inds.indices)
141+
function OffsetArray{T}(init::ArrayInitializer, inds::CartesianIndices{N}) where {T,N}
142+
OffsetArray{T, N}(init, convert(Tuple{Vararg{AbstractUnitRange{Int}}}, inds))
143+
end
142144

143145
Base.IndexStyle(::Type{OA}) where {OA<:OffsetArray} = IndexStyle(parenttype(OA))
144146
parenttype(::Type{OffsetArray{T,N,AA}}) where {T,N,AA} = AA

src/utils.jl

Lines changed: 3 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -16,29 +16,6 @@ _uncolonindices(ax::Tuple, inds::Tuple) = (first(inds), _uncolonindices(tail(ax)
1616
_uncolonindices(ax::Tuple, inds::Tuple{Colon, Vararg{Any}}) = (first(ax), _uncolonindices(tail(ax), tail(inds))...)
1717
_uncolonindices(::Tuple{}, ::Tuple{}) = ()
1818

19-
# Specify offsets using CartesianIndices (issue #71)
20-
# Support a mix of AbstractUnitRanges and CartesianIndices{1}
21-
# Extract the range r from CartesianIndices((r,))
22-
function _stripCartesianIndices(inds::Tuple{CartesianIndices{1},Vararg{Any}})
23-
I = first(inds)
24-
Ir = convert(Tuple{AbstractUnitRange{Int}}, I) |> first
25-
(Ir, _stripCartesianIndices(tail(inds))...)
26-
end
27-
_stripCartesianIndices(inds::Tuple)= (first(inds), _stripCartesianIndices(tail(inds))...)
28-
_stripCartesianIndices(::Tuple{}) = ()
29-
30-
# Support an arbitrary CartesianIndices alongside colons and ranges
31-
# The total number of indices should equal ndims(arr)
32-
# We split the CartesianIndices{N} into N CartesianIndices{1} indices to facilitate dispatch
33-
_splitCartesianIndices(c::CartesianIndices{0}) = ()
34-
function _splitCartesianIndices(c::CartesianIndices)
35-
c1, ct = Base.IteratorsMD.split(c, Val(1))
36-
(c1, _splitCartesianIndices(ct)...)
37-
end
38-
function _splitCartesianIndices(t::Tuple{CartesianIndices, Vararg{Any}})
39-
(_splitCartesianIndices(first(t))..., _splitCartesianIndices(tail(t))...)
40-
end
41-
function _splitCartesianIndices(t::Tuple)
42-
(first(t), _splitCartesianIndices(tail(t))...)
43-
end
44-
_splitCartesianIndices(::Tuple{}) = ()
19+
_expandCartesianIndices(inds::Tuple{<:CartesianIndices, Vararg{Any}}) = (convert(Tuple{Vararg{AbstractUnitRange{Int}}}, inds[1])..., _expandCartesianIndices(Base.tail(inds))...)
20+
_expandCartesianIndices(inds::Tuple{Any,Vararg{Any}}) = (inds[1], _expandCartesianIndices(Base.tail(inds))...)
21+
_expandCartesianIndices(::Tuple{}) = ()

0 commit comments

Comments
 (0)