Deprecate slicedim(…) to copy(selectdim(…)); always return a view (#2…

…6009)

Fixes #18326

NEWS.md

@@ -334,10 +334,6 @@ This section lists changes that do not have deprecation warnings.
     longer the case; now bindings will only exist for packages brought into scope by
     typing `using Package` or `import Package` ([#17997]).
 
-  * `slicedim(b::BitVector, 1, x)` now consistently returns the same thing that `b[x]` would,
-    consistent with its documentation. Previously it would return a `BitArray{0}` for scalar
-    `x` ([#20233]).
-
   * The rules for mixed-signedness integer arithmetic (e.g. `Int32(1) + UInt64(1)`) have been
     simplified: if the arguments have different sizes (in bits), then the type of the larger
     argument is used. If the arguments have the same size, the unsigned type is used ([#9292]).
@@ -651,6 +647,13 @@ Deprecated or removed
   * Using Bool values directly as indices is now deprecated and will be an error in the future. Convert
     them to `Int` before indexing if you intend to access index `1` for `true` and `0` for `false`.
 
+  * `slicedim(A, d, i)` has been deprecated in favor of `copy(selectdim(A, d, i))`. The new
+    `selectdim` function now always returns a view into `A`; in many cases the `copy` is
+    not necessary. Previously, `slicedim` on a vector `V` over dimension `d=1` and scalar
+	index `i` would return the just selected element (unless `V` was a `BitVector`). This
+	has now been made consistent: `selectdim` now always returns a view into the original
+	array, with a zero-dimensional view in this specific case ([#26009]).
+
   * `whos` has been renamed `varinfo`, and now returns a markdown table instead of printing
     output ([#12131]).
 

base/abstractarraymath.jl

@@ -98,10 +98,11 @@ imag(x::AbstractArray{<:Real}) = zero(x)
 # index A[:,:,...,i,:,:,...] where "i" is in dimension "d"
 
 """
-    slicedim(A, d::Integer, i)
+    selectdim(A, d::Integer, i)
 
-Return all the data of `A` where the index for dimension `d` equals `i`. Equivalent to
-`A[:,:,...,i,:,:,...]` where `i` is in position `d`.
+Return a view of all the data of `A` where the index for dimension `d` equals `i`.
+
+Equivalent to `view(A,:,:,...,i,:,:,...)` where `i` is in position `d`.
 
 # Examples
 ```jldoctest
@@ -110,17 +111,18 @@ julia> A = [1 2 3 4; 5 6 7 8]
  1  2  3  4
  5  6  7  8
 
-julia> slicedim(A,2,3)
-2-element Array{Int64,1}:
+julia> selectdim(A, 2, 3)
+2-element view(::Array{Int64,2}, Base.OneTo(2), 3) with eltype Int64:
  3
  7
 ```
 """
-function slicedim(A::AbstractArray, d::Integer, i)
+@inline selectdim(A::AbstractArray, d::Integer, i) = _selectdim(A, d, i, setindex(axes(A), i, d))
+@noinline function _selectdim(A, d, i, idxs)
     d >= 1 || throw(ArgumentError("dimension must be ≥ 1"))
     nd = ndims(A)
-    d > nd && (i == 1 || throw_boundserror(A, (ntuple(k->Colon(),nd)..., ntuple(k->1,d-1-nd)..., i)))
-    A[setindex(axes(A), i, d)...]
+    d > nd && (i == 1 || throw(BoundsError(A, (ntuple(k->Colon(),d-1)..., i))))
+    return view(A, idxs...)
 end
 
 """
@@ -167,7 +169,7 @@ function flipdim(A::AbstractArray, d::Integer)
     let B=B # workaround #15276
         alli = [ axes(B,n) for n in 1:nd ]
         for i in indsd
-            B[[ n==d ? sd-i : alli[n] for n in 1:nd ]...] = slicedim(A, d, i)
+            B[[ n==d ? sd-i : alli[n] for n in 1:nd ]...] = selectdim(A, d, i)
         end
     end
     return B

base/deprecated.jl

@@ -1360,6 +1360,20 @@ end
 # issue #25928
 @deprecate wait(t::Task) fetch(t)
 
+# issue #18326
+@deprecate slicedim(A::AbstractArray, d::Integer, i) copy(selectdim(A, d, i))
+@deprecate slicedim(A::BitVector, d::Integer, i::Number) copy(selectdim(A, d, i))
+function slicedim(A::AbstractVector, d::Integer, i::Number)
+    if d == 1
+        # slicedim would have returned a scalar value, selectdim always returns views
+        depwarn("`slicedim(A::AbstractVector, d::Integer, i)` is deprecated, use `selectdim(A, d, i)[]` instead.", :slicedim)
+        selectdim(A, d, i)[]
+    else
+        depwarn("`slicedim(A::AbstractArray, d::Integer, i)` is deprecated, use `copy(selectdim(A, d, i))` instead.", :slicedim)
+        copy(selectdim(A, d, i))
+    end
+end
+
 # PR 25062
 @deprecate(link_pipe(pipe; julia_only_read = true, julia_only_write = true),
            link_pipe!(pipe, reader_supports_async = julia_only_read, writer_supports_async = julia_only_write),

base/exports.jl

@@ -431,7 +431,7 @@ export
     shuffle,
     shuffle!,
     size,
-    slicedim,
+    selectdim,
     sort!,
     sort,
     sortcols,

doc/src/base/arrays.md

@@ -102,7 +102,7 @@ Base.@view
 Base.@views
 Base.parent
 Base.parentindices
-Base.slicedim
+Base.selectdim
 Base.reinterpret
 Base.reshape
 Base.squeeze

test/arrayops.jl

@@ -1806,16 +1806,18 @@ end
 # range, range ops
 @test (1:5) + (1.5:5.5) == 2.5:2.0:10.5
 
-@testset "slicedim" begin
+@testset "selectdim" begin
+    f26009(A, i) = selectdim(A, 1, i)
     for A in (reshape(Vector(1:20), 4, 5),
               reshape(1:20, 4, 5))
         local A
-        @test slicedim(A, 1, 2) == 2:4:20
-        @test slicedim(A, 2, 2) == 5:8
-        @test_throws ArgumentError slicedim(A,0,1)
-        @test slicedim(A, 3, 1) == A
-        @test_throws BoundsError slicedim(A, 3, 2)
-        @test @inferred(slicedim(A, 1, 2:2)) == Vector(2:4:20)'
+        @test selectdim(A, 1, 2) == 2:4:20
+        @test selectdim(A, 2, 2) == 5:8
+        @test_throws ArgumentError selectdim(A,0,1)
+        @test selectdim(A, 3, 1) == A
+        @test_throws BoundsError selectdim(A, 3, 2)
+        @test @inferred(f26009(A, 2:2)) == reshape(2:4:20, 1, :)
+        @test @inferred(f26009(A, 2)) == 2:4:20
     end
 end
 

test/bitarray.jl

@@ -5,6 +5,7 @@ using Random, LinearAlgebra, Test
 
 tc(r1::NTuple{N,Any}, r2::NTuple{N,Any}) where {N} = all(x->tc(x...), [zip(r1,r2)...])
 tc(r1::BitArray{N}, r2::Union{BitArray{N},Array{Bool,N}}) where {N} = true
+tc(r1::SubArray{Bool,N1,BitArray{N2}}, r2::SubArray{Bool,N1,<:Union{BitArray{N2},Array{Bool,N2}}}) where {N1,N2} = true
 tc(r1::Transpose{Bool,BitVector}, r2::Union{Transpose{Bool,BitVector},Transpose{Bool,Vector{Bool}}}) = true
 tc(r1::T, r2::T) where {T} = true
 tc(r1,r2) = false
@@ -18,7 +19,7 @@ function check_bitop_call(ret_type, func, args...)
     ret_type ≢ nothing && !isa(r1, ret_type) && @show ret_type, r1
     ret_type ≢ nothing && @test isa(r1, ret_type)
     @test tc(r1, r2)
-    @test isequal(r1, ret_type ≡ nothing ? r2 : convert(ret_type, r2))
+    @test isequal(r1, ret_type ≡ nothing ? r2 : r2)
     @test bitcheck(r1)
 end
 macro check_bit_operation(ex, ret_type)
@@ -1033,7 +1034,7 @@ timesofar("binary comparison")
     for d = 1:4
         j = rand(1:size(b1, d))
         #for j = 1 : size(b1, d)
-            @check_bit_operation slicedim(b1, d, j) BitArray{3}
+            @check_bit_operation selectdim(b1, d, j) SubArray{Bool, 3, BitArray{4}}
         #end
         @check_bit_operation flipdim(b1, d) BitArray{4}
     end
@@ -1071,9 +1072,9 @@ timesofar("binary comparison")
         i2 = circshift!(b1, -j)
         @test b2 == i2
 
-        @check_bit_operation slicedim(b1, 1, m) Bool
+        @check_bit_operation selectdim(b1, 1, m) SubArray{Bool, 0}
     end
-    @check_bit_operation slicedim(b1, 1, :) BitVector
+    @check_bit_operation selectdim(b1, 1, :) SubArray{Bool, 1}
 end
 
 timesofar("datamove")