Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Feb 14, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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]).
Expand Down Expand Up @@ -646,6 +642,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]).

Expand Down
20 changes: 11 additions & 9 deletions base/abstractarraymath.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

"""
Expand Down Expand Up @@ -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
Expand Down
14 changes: 14 additions & 0 deletions base/deprecated.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion base/exports.jl
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ export
shuffle,
shuffle!,
size,
slicedim,
selectdim,
sort!,
sort,
sortcols,
Expand Down
2 changes: 1 addition & 1 deletion doc/src/base/arrays.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ Base.@view
Base.@views
Base.parent
Base.parentindices
Base.slicedim
Base.selectdim
Base.reinterpret
Base.reshape
Base.squeeze
Expand Down
16 changes: 9 additions & 7 deletions test/arrayops.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
9 changes: 5 additions & 4 deletions test/bitarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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")
Expand Down