Stricter BlockRange constructors (#472)

Closes #471.

This removes constructors `BlockRange()`, `BlockRange(2, 2)`, and
`BlockRange(1:2, 1:2)`. Instead, the more explicit versions
`BlockRange(())`, `BlockRange((2, 2))`, and `BlockRange((1:2, 1:2))` can
be used.

This now makes `BlockRange` constructors more consistent with
`CartesianIndices` constructors:
```julia
julia> CartesianIndices()
ERROR: MethodError: no method matching CartesianIndices()
The type `CartesianIndices` exists, but no method is defined for this combination of argument types when trying to construct it.

Closest candidates are:
  CartesianIndices(::Tuple{})
   @ Base multidimensional.jl:271
  CartesianIndices(::AbstractArray)
   @ Base multidimensional.jl:281
  CartesianIndices(::R) where {N, R<:NTuple{N, OrdinalRange{Int64, Int64}}}
   @ Base multidimensional.jl:268
  ...

Stacktrace:
 [1] top-level scope
   @ REPL[9]:1
 [2] top-level scope
   @ none:1

julia> CartesianIndices(2, 2)
ERROR: MethodError: no method matching CartesianIndices(::Int64, ::Int64)
The type `CartesianIndices` exists, but no method is defined for this combination of argument types when trying to construct it.

Closest candidates are:
  CartesianIndices(::Tuple{})
   @ Base multidimensional.jl:271
  CartesianIndices(::AbstractArray)
   @ Base multidimensional.jl:281
  CartesianIndices(::R) where {N, R<:NTuple{N, OrdinalRange{Int64, Int64}}}
   @ Base multidimensional.jl:268
  ...

Stacktrace:
 [1] top-level scope
   @ REPL[12]:1
 [2] top-level scope
   @ none:1

julia> CartesianIndices(1:2, 1:2)
ERROR: MethodError: no method matching CartesianIndices(::UnitRange{Int64}, ::UnitRange{Int64})
The type `CartesianIndices` exists, but no method is defined for this combination of argument types when trying to construct it.

Closest candidates are:
  CartesianIndices(::AbstractArray)
   @ Base multidimensional.jl:281
  CartesianIndices(::Tuple{})
   @ Base multidimensional.jl:271
  CartesianIndices(::R) where {N, R<:NTuple{N, OrdinalRange{Int64, Int64}}}
   @ Base multidimensional.jl:268
  ...

Stacktrace:
 [1] top-level scope
   @ REPL[13]:1
 [2] top-level scope
   @ none:1

julia> CartesianIndices(())
CartesianIndices(())

julia> CartesianIndices((2, 2))
CartesianIndices((2, 2))

julia> CartesianIndices((1:2, 1:2))
CartesianIndices((1:2, 1:2))
```
and removes the inconsistency pointed out in #471 when calling
`BlockRange(::AbstractUnitRange)` vs. `BlockRange(::AbstractVector)`:
```julia
julia> using BlockArrays

julia> collect(BlockRange(blockedrange([2, 3])))
5-element Vector{Block{1, Int64}}:
 Block(1)
 Block(2)
 Block(3)
 Block(4)
 Block(5)

julia> collect(BlockRange(mortar([1:2, 3:5])))
2-element Vector{Block{1, Int64}}:
 Block(1)
 Block(2)
```

This is technically breaking, though I would argue that it is a bug fix
because of the inconsistency of the current code design (i.e. I would
argue that the previous behavior of `BlockRange(::AbstractUnitRange)`
was incorrect, and therefore was a bug). I'm curious to see if this
leads to any downstream failures.

---------

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

Author: mtfishman

Project.toml

@@ -2,7 +2,6 @@ name = "BlockArrays"
 uuid = "8e7c35d0-a365-5155-bbbb-fb81a777f24e"
 version = "1.7.0"
 
-
 [deps]
 ArrayLayouts = "4c555306-a7a7-4459-81d9-ec55ddd5c99a"
 BandedMatrices = "aae01518-5342-5314-be14-df237901396f"

docs/src/man/blockarrays.md

@@ -151,7 +151,7 @@ julia> view(A, Block(2)) .= [3,4]; A[Block(2)]
  4.0
 
 julia> view(A, Block.(1:2))
-3-element view(::BlockVector{Float64, Vector{Vector{Float64}}, Tuple{BlockedOneTo{Int64, ArrayLayouts.RangeCumsum{Int64, UnitRange{Int64}}}}}, BlockSlice(BlockRange(1:2),1:1:3)) with eltype Float64 with indices BlockedOneTo([1, 3]):
+3-element view(::BlockVector{Float64, Vector{Vector{Float64}}, Tuple{BlockedOneTo{Int64, ArrayLayouts.RangeCumsum{Int64, UnitRange{Int64}}}}}, BlockSlice(BlockRange((1:2,)),1:1:3)) with eltype Float64 with indices BlockedOneTo([1, 3]):
  1.0
  3.0
  4.0

src/abstractblockarray.jl

@@ -92,7 +92,7 @@ end
 
 # linear block indexing
 @inline function blockcheckbounds(::Type{Bool}, A::AbstractArray, i)
-    blockcheckindex(Bool, BlockRange(blocklength(A)), i)
+    blockcheckindex(Bool, BlockRange((blocklength(A),)), i)
 end
 # cartesian block indexing
 @inline function blockcheckbounds(::Type{Bool}, A::AbstractArray, i...)
@@ -118,7 +118,7 @@ The actual bounds-checking is performed by [`blockcheckindex`](@ref).
 julia> B = BlockArray(zeros(6,6), 1:3, 1:3);
 
 julia> blockaxes(B)
-(BlockRange(Base.OneTo(3)), BlockRange(Base.OneTo(3)))
+(BlockRange((3,)), BlockRange((3,)))
 
 julia> BlockArrays.blockcheckbounds_indices(Bool, blockaxes(B), (1,2))
 true

src/blockaxis.jl

@@ -289,7 +289,7 @@ julia> A = BlockArray([1,2,3],[2,1])
  3
 
 julia> blockaxes(A)
-(BlockRange(Base.OneTo(2)),)
+(BlockRange((2,)),)
 
 julia> B = BlockArray(zeros(3,4), [1,2], [1,2,1])
 2×3-blocked 3×4 BlockMatrix{Float64}:
@@ -299,7 +299,7 @@ julia> B = BlockArray(zeros(3,4), [1,2], [1,2,1])
  0.0  │  0.0  0.0  │  0.0
 
 julia> blockaxes(B)
-(BlockRange(Base.OneTo(2)), BlockRange(Base.OneTo(3)))
+(BlockRange((2,)), BlockRange((3,)))
 ```
 """
 blockaxes(b::AbstractBlockedUnitRange) = _blockaxes(blocklasts(b))
@@ -322,7 +322,7 @@ julia> A = BlockArray([1,2,3], [2,1])
  3
 
 julia> blockaxes(A,1)
-BlockRange(Base.OneTo(2))
+BlockRange((2,))
 
 julia> blockaxes(A,1) |> collect
 2-element Vector{Block{1, Int64}}:

src/blockindices.jl

@@ -324,8 +324,8 @@ end
 
 # deleted code that isn't used, such as 0-dimensional case
 """
-    BlockRange(axes::Tuple{AbstractUnitRange{Int}})
-    BlockRange(sizes::Vararg{Integer})
+    BlockRange(axes::Tuple{Vararg{AbstractUnitRange{<:Integer}}})
+    BlockRange(sizes::Tuple{Vararg{Integer}})
 
 Represent a Cartesian range of blocks.
 
@@ -334,18 +334,18 @@ The relationship between `Block` and `BlockRange` mimics the relationship betwee
 
 # Examples
 ```jldoctest
-julia> BlockRange(2:3, 3:4) |> collect
+julia> BlockRange((2:3, 3:4)) |> collect
 2×2 Matrix{Block{2, Int64}}:
  Block(2, 3)  Block(2, 4)
  Block(3, 3)  Block(3, 4)
 
-julia> BlockRange(2, 2) |> collect # number of elements, starting at 1
+julia> BlockRange((2, 2)) |> collect # number of elements, starting at 1
 2×2 Matrix{Block{2, Int64}}:
  Block(1, 1)  Block(1, 2)
  Block(2, 1)  Block(2, 2)
 
 julia> Block(1):Block(2)
-BlockRange(1:2)
+BlockRange((1:2,))
 ```
 """
 BlockRange
@@ -360,11 +360,8 @@ end
 
 BlockRange(inds::Tuple{Vararg{AbstractUnitRange{<:Integer}}}) =
     BlockRange{length(inds),typeof(inds)}(inds)
-BlockRange(inds::Vararg{AbstractUnitRange{<:Integer}}) = BlockRange(inds)
 
-BlockRange() = BlockRange(())
 BlockRange(sizes::Tuple{Integer, Vararg{Integer}}) = BlockRange(map(oneto, sizes))
-BlockRange(sizes::Vararg{Integer}) = BlockRange(sizes)
 
 BlockRange(B::AbstractArray) = BlockRange(blockaxes(B))
 
@@ -433,13 +430,13 @@ _in(b, ::Tuple{}, ::Tuple{}, ::Tuple{}) = b
 @inline _in(b, i, start, stop) = _in(b & (start[1] <= i[1] <= stop[1]), tail(i), tail(start), tail(stop))
 
 # We sometimes need intersection of BlockRange to return a BlockRange
-intersect(a::BlockRange{1}, b::BlockRange{1}) = BlockRange(intersect(a.indices[1], b.indices[1]))
+intersect(a::BlockRange{1}, b::BlockRange{1}) = BlockRange((intersect(a.indices[1], b.indices[1]),))
 
 # needed for scalar-like broadcasting
 
 BlockSlice{Block{1,BT},T,RT}(a::Base.OneTo) where {BT,T,RT<:AbstractUnitRange} =
     BlockSlice(Block(convert(BT, 1)), convert(RT, a))::BlockSlice{Block{1,BT},T,RT}
 BlockSlice{BlockRange{1,Tuple{BT}},T,RT}(a::Base.OneTo) where {BT<:AbstractUnitRange,T,RT<:AbstractUnitRange} =
-    BlockSlice(BlockRange(convert(BT, Base.OneTo(1))), convert(RT, a))::BlockSlice{BlockRange{1,Tuple{BT}},T,RT}
+    BlockSlice(BlockRange((convert(BT, Base.OneTo(1)),)), convert(RT, a))::BlockSlice{BlockRange{1,Tuple{BT}},T,RT}
 BlockSlice{BlockIndexRange{1,Tuple{BT},I,BI},T,RT}(a::Base.OneTo) where {BT<:AbstractUnitRange,T,RT<:AbstractUnitRange,I,BI} =
     BlockSlice(BlockIndexRange(Block(BI(1)), convert(BT, Base.OneTo(1))), convert(RT, a))::BlockSlice{BlockIndexRange{1,Tuple{BT},I,BI},T,RT}

src/show.jl

@@ -212,7 +212,13 @@ end
 
 # BlockRange
 
-Base.show(io::IO, br::BlockRange) = print(io, "BlockRange(", join(br.indices, ", "), ")")
+function Base.show(io::IO, br::BlockRange)
+    print(io, "BlockRange(")
+    show(io, map(_xform_index, br.indices))
+    print(io, ")")
+end
+_xform_index(i) = i
+_xform_index(i::Base.OneTo) = i.stop
 Base.show(io::IO, ::MIME"text/plain", br::BlockRange) = show(io, br)
 
 # AbstractBlockedUnitRange

src/views.jl

@@ -123,7 +123,7 @@ end
 # this is loosely based on Slice reindex in subarray.jl
 @propagate_inbounds reindex(idxs::Tuple{BlockSlice{<:BlockRange}, Vararg{Any}},
         subidxs::Tuple{BlockSlice{<:BlockRange}, Vararg{Any}}) =
-    (BlockSlice(BlockRange(idxs[1].block.indices[1][Int.(subidxs[1].block)]),
+    (BlockSlice(BlockRange((idxs[1].block.indices[1][Int.(subidxs[1].block)],)),
                                             idxs[1].indices[subidxs[1].block]),
                                 reindex(tail(idxs), tail(subidxs))...)
 

test/test_blockindices.jl

@@ -159,10 +159,14 @@ import BlockArrays: BlockIndex, BlockIndexRange, BlockSlice, BlockedSlice
         @test sprint(show, "text/plain", BlockIndex((1,2), (3,4))) == "Block(1, 2)[3, 4]"
         @test sprint(show, "text/plain", BlockArrays.BlockIndexRange(Block(1), 3:4)) == "Block(1)[3:4]"
 
-        @test sprint(show, "text/plain", BlockRange()) == "BlockRange()"
-        @test sprint(show, "text/plain", BlockRange(1:2)) == "BlockRange(1:2)"
-        @test sprint(show, "text/plain", BlockRange(1:2, 2:3)) == "BlockRange(1:2, 2:3)"
-        @test sprint(show, BlockRange(1:2, 2:3)) == "BlockRange(1:2, 2:3)"
+        @test sprint(show, "text/plain", BlockRange(())) == "BlockRange(())"
+        @test sprint(show, "text/plain", BlockRange((1:2,))) == "BlockRange((1:2,))"
+        @test sprint(show, "text/plain", BlockRange((2,))) == "BlockRange((2,))"
+        @test sprint(show, "text/plain", BlockRange((Base.OneTo(2),))) == "BlockRange((2,))"
+        @test sprint(show, "text/plain", BlockRange((1:2, 2:3,))) == "BlockRange((1:2, 2:3))"
+        @test sprint(show, "text/plain", BlockRange((2, 3,))) == "BlockRange((2, 3))"
+        @test sprint(show, "text/plain", BlockRange(Base.OneTo.((2, 3)))) == "BlockRange((2, 3))"
+        @test sprint(show, BlockRange((1:2, 2:3))) == "BlockRange((1:2, 2:3))"
     end
 end
 
@@ -220,7 +224,7 @@ end
         b = BlockRange(OffsetArrays.IdOffsetRange.((2:4, 3:5), 2))
         @test b[axes(b)...] === b
 
-        b = BlockRange(3)
+        b = BlockRange((3,))
         for i in 1:3
             @test b[i] == Block(i)
         end
@@ -465,7 +469,7 @@ end
         b = BlockRange(OffsetArrays.IdOffsetRange.((2:4, 3:5), 2))
         @test b[axes(b)...] === b
 
-        b = BlockRange(3)
+        b = BlockRange((3,))
         for i in 1:3
             @test b[i] == Block(i)
         end

test/test_blockrange.jl

@@ -4,9 +4,15 @@ using BlockArrays, Test
 
 @testset "block range" begin
     # test backend code
-    @test BlockRange((1:3),) == BlockRange{1,Tuple{UnitRange{Int}}}((1:3,))
-    @test BlockRange(1:3) == BlockRange((1:3),)
+    @test BlockRange((1:3,)) == BlockRange{1,Tuple{UnitRange{Int}}}((1:3,))
+    @test BlockRange(1:3) === BlockRange(Base.OneTo(1))
+    @test BlockRange(blockedrange([2,3])) === BlockRange((Base.OneTo(2),))
+    @test BlockRange(blockedrange(2,[2,3])) === BlockRange((Base.OneTo(2),))
     @test_throws ArgumentError Block(1,1):Block(2,2)
+    @test_throws MethodError BlockRange(1:3, 1:3)
+    @test_throws MethodError BlockRange(3)
+    @test_throws MethodError BlockRange(3, 3)
+    @test_throws MethodError BlockRange()
 
     @test eltype(Block.(1:2)) == Block{1,Int}
     @test eltype(typeof(Block.(1:2))) == Block{1,Int}
@@ -78,9 +84,9 @@ using BlockArrays, Test
     @test V ==  A[Block.(1:2), Block.(2:3)]
 
     @testset "iterator" begin
-        @test BlockRange()[] == collect(BlockRange())[] == Block()
-        @test BlockRange(1:3) == collect(BlockRange(1:3)) == [Block(1),Block(2),Block(3)]
-        @test BlockRange(1:3,1:2) == collect(BlockRange(1:3,1:2))
+        @test BlockRange(())[] == collect(BlockRange(()))[] == Block()
+        @test BlockRange((1:3,)) == collect(BlockRange((1:3,))) == [Block(1),Block(2),Block(3)]
+        @test BlockRange((1:3,1:2)) == collect(BlockRange((1:3,1:2)))
     end
 
     # non Int64 range