Rename BlockedSlice from JuliaArrays#459 to NoncontiguousBlockSlice (JuliaArrays#479)

This PR is split off from JuliaArrays#462, it renames `BlockedSlice` introduced in
JuliaArrays#459 to `NoncontiguousBlockSlice` and also adds more functionality and
tests so that it is as featureful as `BlockSlice`. The reason for the
name change is that in JuliaArrays#462, this type will be used in slicing
operations such as `V[[Block(1), Block(3)]]` and `V[Block(1)[[1, 3]]]`.
In my opinion, `BlockedSlice` doesn't make a lot of sense as a name for
the latter case. The more general concept is that it is a slice object
constructed when there is a non-contiguous blockwise slice of some form
(either non-contiguous blocks or non-contiguous within a block).

I'm open to other name suggestions, `NoncontiguousBlockSlice` is the
best I could come up with and there isn't really an analogous case for
this in Base from what I've seen. Note the main reason why we can't just
use `BlockSlice` for those cases is that `BlockSlice` is an
`AbstractUnitRange` subtype and `NoncontiguousBlockSlice` covers cases
where the slices aren't ranges, so maybe `BlockSliceVector` could work
as a name to emphasize that point.

---------

Co-authored-by: Sheehan Olver <[email protected]>

Author: mtfishman

docs/src/lib/internals.md

@@ -25,6 +25,7 @@ BlockRange
 BlockIndexRange
 BlockIndices
 BlockSlice
+NoncontiguousBlockSlice
 unblock
 SubBlockIterator
 blockcheckbounds_indices

src/blockindices.jl

@@ -419,26 +419,39 @@ _indices(B) = B
 @propagate_inbounds view(C::CartesianIndices{N}, bs::Vararg{BlockSlice,N}) where {N} = view(C, map(x->x.indices, bs)...)
 
 """
-    BlockedSlice(blocks, indices)
+    NoncontiguousBlockSlice(blocks, indices)
 
-Represents blocked indices attached to a collection of corresponding blocks.
+Represents an AbstractVector of indices attached to a (potentially non-contiguous) subblock,
+set of blocks, or set of subblocks. This is the generalization of `BlockSlice` to
+non-contiguous slices.
 
-Upon calling `to_indices()`, a collection of blocks are converted to BlockedSlice objects to represent
+Upon calling `to_indices()`, a collection of blocks are converted to NoncontiguousBlockSlice objects to represent
 the indices over which the blocks span.
 
 This mimics the relationship between `Colon` and `Base.Slice`, `Block` and `BlockSlice`, etc.
 """
-struct BlockedSlice{BB,T<:Integer,INDS<:AbstractVector{T}} <: AbstractVector{T}
-    blocks::BB
+struct NoncontiguousBlockSlice{BB,T,INDS<:AbstractVector{T}} <: AbstractVector{T}
+    block::BB
     indices::INDS
 end
 
-for f in (:axes, :size)
-    @eval $f(S::BlockedSlice) = $f(S.indices)
+Block(bs::NoncontiguousBlockSlice{<:Block}) = bs.block
+Block(bs::NoncontiguousBlockSlice{<:BlockIndexRange}) = Block(bs.block)
+
+for f in (:axes, :unsafe_indices, :axes1, :first, :last, :size, :length,
+          :unsafe_length, :start)
+    @eval $f(S::NoncontiguousBlockSlice) = $f(S.indices)
 end
 
-@propagate_inbounds getindex(S::BlockedSlice, i::Integer) = getindex(S.indices, i)
-@propagate_inbounds getindex(S::BlockedSlice, k::Block{1}) = BlockSlice(S.blocks[Int(k)], getindex(S.indices, k))
+_indices(B::NoncontiguousBlockSlice) = B.indices
+
+@propagate_inbounds getindex(S::NoncontiguousBlockSlice, i::Integer) = getindex(S.indices, i)
+@propagate_inbounds getindex(S::NoncontiguousBlockSlice{<:Block{1}}, k::AbstractVector{<:Integer}) =
+    NoncontiguousBlockSlice(S.block[_indices(k)], S.indices[_indices(k)])
+@propagate_inbounds getindex(S::NoncontiguousBlockSlice{<:BlockIndexRange{1,<:Tuple{AbstractVector}}}, k::AbstractVector{<:Integer}) =
+    NoncontiguousBlockSlice(S.block[_indices(k)], S.indices[_indices(k)])
+@propagate_inbounds getindex(S::NoncontiguousBlockSlice{<:AbstractVector{<:Block{1}}}, k::Block{1}) =
+    BlockSlice(S.block[Int(k)], getindex(S.indices, k))
 
 struct BlockRange{N,R<:NTuple{N,AbstractUnitRange{<:Integer}}} <: AbstractArray{Block{N,Int},N}
     indices::R

src/views.jl

@@ -11,7 +11,7 @@ function unblock(A, inds, I)
 end
 
 _blockslice(B, a::AbstractUnitRange) = BlockSlice(B, a)
-_blockslice(B, a) = BlockedSlice(B, a)
+_blockslice(B, a) = NoncontiguousBlockSlice(B, a)
 
 # Allow `ones(2)[Block(1)[1:1], Block(1)[1:1]]` which is
 # similar to `ones(2)[1:1, 1:1]`.
@@ -209,11 +209,11 @@ block(A::Block) = A
 @inline view(block_arr::AbstractBlockArray{<:Any,N}, blocks::Vararg{BlockSlice1, N}) where N =
     view(block_arr, map(block,blocks)...)
 
-const BlockSlices = Union{Base.Slice,BlockSlice{<:BlockRange{1}},BlockedSlice}
+const BlockSlices = Union{Base.Slice,BlockSlice{<:BlockRange{1}},NoncontiguousBlockSlice{<:AbstractVector{<:Block{1}}}}
 # view(V::SubArray{<:Any,N,NTuple{N,BlockSlices}},
 
 _block_reindex(b::BlockSlice, i::Block{1}) = b.block[Int(i)]
-_block_reindex(b::BlockedSlice, i::Block{1}) = b.blocks[Int(i)]
+_block_reindex(b::NoncontiguousBlockSlice, i::Block{1}) = b.block[Int(i)]
 _block_reindex(b::Slice, i::Block{1}) = i
 
 @inline view(V::SubArray{<:Any,N,<:AbstractBlockArray,<:NTuple{N,BlockSlices}}, block::Block{N}) where N =

test/test_blockindices.jl

@@ -2,7 +2,7 @@ module TestBlockIndices
 
 using BlockArrays, FillArrays, Test, StaticArrays, ArrayLayouts
 using OffsetArrays
-import BlockArrays: BlockIndex, BlockIndexRange, BlockSlice, BlockedSlice
+import BlockArrays: BlockIndex, BlockIndexRange, BlockSlice, NoncontiguousBlockSlice
 
 @testset "Blocks" begin
     @test Int(Block(2)) === Integer(Block(2)) === Number(Block(2)) === 2
@@ -882,6 +882,9 @@ end
     @test b[1:2] ≡ b[1:2][1:2] ≡ BlockSlice(Block(5)[1:2],1:2)
     @test Block(b) ≡ Block(5)
 
+    bi = BlockSlice(Block(2)[2:4],3:5)
+    @test Block(bi) ≡ Block(2)
+    @test bi[2:3] ≡ BlockSlice(Block(2)[3:4],4:5)
     @test Block(BlockSlice(Block(2)[1:2], 3:4)) ≡ Block(2)
 
     @testset "OneTo converts" begin
@@ -901,14 +904,24 @@ end
     end
 end
 
-@testset "BlockedSlice" begin
-    b = BlockedSlice([Block(2), Block(1)], mortar([3:5, 1:2]))
-    @test length(b) == 5
-    for i in eachindex(b.indices)
-        @test b[i] === b.indices[i]
+@testset "NoncontiguousBlockSlice" begin
+    bs = NoncontiguousBlockSlice([Block(2),Block(1)], mortar([3:5,1:2]))
+    @test length(bs) ≡ 5
+    for i in eachindex(bs.indices)
+        @test bs[i] ≡ bs.indices[i]
     end
-    @test b[Block(1)] === BlockSlice(Block(2), 3:5)
-    @test b[Block(2)] === BlockSlice(Block(1), 1:2)
+    @test bs[Block(1)] ≡ BlockSlice(Block(2), 3:5)
+    @test bs[Block(2)] ≡ BlockSlice(Block(1), 1:2)
+    @test BlockArrays._indices(bs) == mortar([3:5,1:2])
+
+    b = NoncontiguousBlockSlice(Block(3), 2:4)
+    @test b[2:3] ≡ NoncontiguousBlockSlice(Block(3)[2:3], 3:4)
+    @test Block(b) ≡ Block(3)
+    @test BlockArrays._indices(b) ≡ 2:4
+
+    bir = NoncontiguousBlockSlice(Block(3)[3:5], 4:6)
+    @test Block(bir) ≡ Block(3)
+    @test bir[2:3] ≡ NoncontiguousBlockSlice(Block(3)[4:5], 5:6)
 end
 
 #=

test/test_blockrange.jl

@@ -32,7 +32,7 @@ using BlockArrays, Test
     V = view(A, [Block(3), Block(2)])
     @test V == [4, 5, 6, 2, 3]
     I = parentindices(V)[1]
-    @test I isa BlockArrays.BlockedSlice{<:Vector{<:Block{1}}}
+    @test I isa BlockArrays.NoncontiguousBlockSlice{<:Vector{<:Block{1}}}
     @test V[Block(1)] == 4:6
     @test V[Block(2)] == 2:3
     @test view(V, Block(1)) === view(A, Block(3))

test/test_blockviews.jl

@@ -2,7 +2,7 @@ module TestBlockViews
 
 using BlockArrays, ArrayLayouts, Test
 using FillArrays
-import BlockArrays: BlockedLogicalIndex
+import BlockArrays: BlockedLogicalIndex, NoncontiguousBlockSlice
 import Base: LogicalIndex
 
 # useds to force SubArray return
@@ -219,6 +219,22 @@ bview(a, b) = Base.invoke(view, Tuple{AbstractArray,Any}, a, b)
         @test A[Block.(2:3)] == A[2:end]
     end
 
+    @testset "getindex and view with Block-vector" begin
+        A = BlockArray(reshape(collect(1:(6*12)),6,12), 1:3, 3:5)
+        V = view(A, [Block(3),Block(2)], [Block(3),Block(2)])
+        @test V[Block(1,1)] == A[Block(3,3)]
+        @test V[Block(2,1)] == A[Block(2,3)]
+        @test V[Block(1,2)] == A[Block(3,2)]
+        @test V[Block(2,2)] == A[Block(2,2)]
+        I = parentindices(V)
+        @test I[1] isa NoncontiguousBlockSlice{<:Vector{<:Block{1}}}
+        @test I[2] isa NoncontiguousBlockSlice{<:Vector{<:Block{1}}}
+        @test view(V, Block(1,1)) === view(A, Block(3,3))
+        @test view(V, Block(2,1)) === view(A, Block(2,3))
+        @test view(V, Block(1,2)) === view(A, Block(3,2))
+        @test view(V, Block(2,2)) === view(A, Block(2,2))
+    end
+
     @testset "non-allocation blocksize" begin
         A = BlockArray(randn(5050), 1:100)
         @test blocksize(A) == (100,)
@@ -316,7 +332,7 @@ bview(a, b) = Base.invoke(view, Tuple{AbstractArray,Any}, a, b)
         @test a[Block(1)[1:3]] ≡ view(a,Block(1)[1:3]) ≡ view(v,Block(1)[1:3]) ≡ 7:9
     end
 
-    @testset "blockrange-of-blockreange" begin
+    @testset "blockrange-of-blockrange" begin
         a = mortar([7:9,5:6])
         v = view(a,Block.(1:2))
         @test view(v, Block(1)) ≡ 7:9