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

RFC: Deprecate multi-value non-scalar indexed assignment #24368

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
1e44d4c
WIP
mbauman Oct 10, 2017
86de444
WIP: SparseArrays done
mbauman Feb 7, 2018
6345653
WIP: LinearAlgebra
mbauman Feb 7, 2018
3efe97b
Fixup BitArray -- removing a specialized method that we cannot perfor…
mbauman Feb 7, 2018
ecb8099
WIP: All but bitarray tests pass now...
mbauman Feb 8, 2018
6662038
Get BitArray passing tests
mbauman Feb 9, 2018
b729d65
Add missing core.jl test fixes
mbauman Feb 9, 2018
3189123
Remove setindex! methods that slipped in through the rebase. Fixup sm…
mbauman Feb 9, 2018
dc26b1d
Fix OffsetArrays: `A[I] = B` didn't care about indices, but `A[I] .= …
mbauman Feb 9, 2018
f95928c
Reimplement optimized setindex!(::Array, ::Array) methods as copyto!
mbauman Feb 9, 2018
995f422
Reset no-longer-broken test_broken test
mbauman Feb 9, 2018
08337d1
Add NEWS.md
mbauman Feb 9, 2018
57fa2c5
Update documentation
mbauman Feb 9, 2018
8921ba0
fixup copyto! ambiguity
mbauman Feb 10, 2018
828afaf
Relax copyto! bounds checks to number of elements
mbauman Feb 12, 2018
fa82fe6
Fixup new bitarray tests
mbauman Feb 12, 2018
1947897
Merge branch 'master' into mb/deprecatemultivaluenonscalarindexedassi…
mbauman Feb 27, 2018
41d458e
Remove new tests that use nonscalar indexed assignment
mbauman Feb 27, 2018
83d99cd
Merge branch 'master' into mb/deprecatemultivaluenonscalarindexedassi…
mbauman Feb 28, 2018
b4d9c6a
Merge branch 'master' into mb/deprecatemultivaluenonscalarindexedassi…
mbauman Mar 1, 2018
95a78df
Merge remote-tracking branch 'origin/master' into mb/deprecatemultiva…
mbauman Mar 5, 2018
50cbdd2
More missed/merged deprecation fixes
mbauman Mar 5, 2018
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
7 changes: 7 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,13 @@ This section lists changes that do not have deprecation warnings.
`AbstractArray` types that specialized broadcasting using the old internal API will
need to switch to the new API. ([#20740])

* Assigning many elements of array `B` to many indices of array `A` with `setindex!(A, B, ...)`
or the `A[...] = B` syntax has been deprecated in favor of broadcasting: `A[...] .= B`. In
making this transformation, be aware that the two behaviors are not perfectly compatible as
broadcasting will additionally treat tuples and other broadcastable collections like arrays.
This also means that custom arrays that extend the `setindex!` function should transform their
specializations to `broadcast!` methods. ([#24368])

* The logging system has been redesigned - `info` and `warn` are deprecated
and replaced with the logging macros `@info`, `@warn`, `@debug` and
`@error`. The `logging` function is also deprecated and replaced with
Expand Down
39 changes: 22 additions & 17 deletions base/abstractarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1015,7 +1015,9 @@ _unsafe_ind2sub(sz, i) = (@_inline_meta; _ind2sub(sz, i))
"""
setindex!(A, X, inds...)

Store values from array `X` within some subset of `A` as specified by `inds`.
Store the value `X` to a location or multiple locations in `A` as specified by `inds`.

See the manual section on [array indexing](@ref man-array-indexing) for details.
"""
function setindex!(A::AbstractArray, v, I...)
@_propagate_inbounds_meta
Expand Down Expand Up @@ -1168,7 +1170,7 @@ get(A::AbstractArray, I::Dims, default) = checkbounds(Bool, A, I...) ? A[I...] :
function get!(X::AbstractVector{T}, A::AbstractVector, I::Union{AbstractRange,AbstractVector{Int}}, default::T) where T
# 1d is not linear indexing
ind = findall(occursin(indices1(A)), I)
X[ind] = A[I[ind]]
X[ind] .= A[I[ind]]
Xind = indices1(X)
X[first(Xind):first(ind)-1] = default
X[last(ind)+1:last(Xind)] = default
Expand All @@ -1177,7 +1179,7 @@ end
function get!(X::AbstractArray{T}, A::AbstractArray, I::Union{AbstractRange,AbstractVector{Int}}, default::T) where T
# Linear indexing
ind = findall(occursin(1:length(A)), I)
X[ind] = A[I[ind]]
X[ind] .= A[I[ind]]
X[1:first(ind)-1] = default
X[last(ind)+1:length(X)] = default
X
Expand All @@ -1188,7 +1190,7 @@ get(A::AbstractArray, I::AbstractRange, default) = get!(similar(A, typeof(defaul
function get!(X::AbstractArray{T}, A::AbstractArray, I::RangeVecIntList, default::T) where T
fill!(X, default)
dst, src = indcopy(size(A), I)
X[dst...] = A[src...]
X[dst...] .= A[src...]
X
end

Expand Down Expand Up @@ -1239,7 +1241,7 @@ function typed_vcat(::Type{T}, V::AbstractVector...) where T
for k=1:length(V)
Vk = V[k]
p1 = pos+length(Vk)-1
a[pos:p1] = Vk
a[pos:p1] .= Vk
pos = p1+1
end
a
Expand Down Expand Up @@ -1275,7 +1277,7 @@ function typed_hcat(::Type{T}, A::AbstractVecOrMat...) where T
for k=1:nargs
Ak = A[k]
p1 = pos+(isa(Ak,AbstractMatrix) ? size(Ak, 2) : 1)-1
B[:, pos:p1] = Ak
B[:, pos:p1] .= Ak
pos = p1+1
end
end
Expand All @@ -1299,7 +1301,7 @@ function typed_vcat(::Type{T}, A::AbstractVecOrMat...) where T
for k=1:nargs
Ak = A[k]
p1 = pos+size(Ak,1)-1
B[pos:p1, :] = Ak
B[pos:p1, :] .= Ak
pos = p1+1
end
return B
Expand Down Expand Up @@ -1377,7 +1379,7 @@ function _cat(A, shape::NTuple{N}, catdims, X...) where N
end
end
I::NTuple{N, UnitRange{Int}} = (inds...,)
A[I...] = x
concatenate_setindex!(A, x, I...)
end
return A
end
Expand Down Expand Up @@ -1576,7 +1578,7 @@ function typed_hvcat(::Type{T}, rows::Tuple{Vararg{Int}}, as::AbstractVecOrMat..
if c-1+szj > nc
throw(ArgumentError("block row $(i) has mismatched number of columns (expected $nc, got $(c-1+szj))"))
end
out[r:r-1+szi, c:c-1+szj] = Aj
out[r:r-1+szi, c:c-1+szj] .= Aj
c += szj
end
if c != nc+1
Expand Down Expand Up @@ -1916,38 +1918,38 @@ function mapslices(f, A::AbstractArray, dims::AbstractVector)
end
nextra = max(0, length(dims)-ndims(r1))
if eltype(Rsize) == Int
Rsize[dims] = [size(r1)..., ntuple(d->1, nextra)...]
Rsize[dims] .= [size(r1)..., ntuple(d->1, nextra)...]
else
Rsize[dims] = [axes(r1)..., ntuple(d->OneTo(1), nextra)...]
Rsize[dims] .= [axes(r1)..., ntuple(d->OneTo(1), nextra)...]
end
R = similar(r1, tuple(Rsize...,))

ridx = Any[map(first, axes(R))...]
for d in dims
ridx[d] = axes(R,d)
ridx[d] = Slice(axes(R,d))
end

R[ridx...] = r1
concatenate_setindex!(R, r1, ridx...)

nidx = length(otherdims)
indices = Iterators.drop(CartesianIndices(itershape), 1)
indices = Iterators.drop(CartesianIndices(itershape), 1) # skip the first element, we already handled it
inner_mapslices!(safe_for_reuse, indices, nidx, idx, otherdims, ridx, Aslice, A, f, R)
end

@noinline function inner_mapslices!(safe_for_reuse, indices, nidx, idx, otherdims, ridx, Aslice, A, f, R)
if safe_for_reuse
# when f returns an array, R[ridx...] = f(Aslice) line copies elements,
# so we can reuse Aslice
for I in indices # skip the first element, we already handled it
for I in indices
replace_tuples!(nidx, idx, ridx, otherdims, I)
_unsafe_getindex!(Aslice, A, idx...)
R[ridx...] = f(Aslice)
concatenate_setindex!(R, f(Aslice), ridx...)
end
else
# we can't guarantee safety (#18524), so allocate new storage for each slice
for I in indices
replace_tuples!(nidx, idx, ridx, otherdims, I)
R[ridx...] = f(A[idx...])
concatenate_setindex!(R, f(A[idx...]), ridx...)
end
end

Expand All @@ -1960,6 +1962,9 @@ function replace_tuples!(nidx, idx, ridx, otherdims, I)
end
end

concatenate_setindex!(R, v, I...) = R[I...] = v
concatenate_setindex!(R, X::AbstractArray, I...) = R[I...] .= X


## 1 argument

Expand Down
10 changes: 5 additions & 5 deletions base/abstractarraymath.jl
Original file line number Diff line number Diff line change
Expand Up @@ -169,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 ]...] = selectdim(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 Expand Up @@ -278,7 +278,7 @@ function repeat(a::AbstractVecOrMat, m::Integer, n::Integer=1)
R = d:d+p-1
for i=1:m
c = (i-1)*o+1
b[c:c+o-1, R] = a
b[c:c+o-1, R] .= a
end
end
return b
Expand All @@ -289,7 +289,7 @@ function repeat(a::AbstractVector, m::Integer)
b = similar(a, o*m)
for i=1:m
c = (i-1)*o+1
b[c:c+o-1] = a
b[c:c+o-1] .= a
end
return b
end
Expand Down Expand Up @@ -378,7 +378,7 @@ cat_fill!(R, X::AbstractArray, inds) = fill!(view(R, inds...), X)

# fill the first inner block
if all(x -> x == 1, inner)
R[axes(A)...] = A
R[axes(A)...] .= A
else
inner_indices = [1:n for n in inner]
for c in CartesianIndices(axes(A))
Expand All @@ -400,7 +400,7 @@ cat_fill!(R, X::AbstractArray, inds) = fill!(view(R, inds...), X)
B = view(R, src_indices...)
for j in 2:outer[i]
dest_indices[i] = dest_indices[i] .+ inner_shape[i]
R[dest_indices...] = B
R[dest_indices...] .= B
end
src_indices[i] = dest_indices[i] = 1:shape[i]
end
Expand Down
44 changes: 0 additions & 44 deletions base/array.jl
Original file line number Diff line number Diff line change
Expand Up @@ -699,50 +699,6 @@ function setindex! end
@eval setindex!(A::Array{T}, x, i1::Int, i2::Int, I::Int...) where {T} =
(@_inline_meta; arrayset($(Expr(:boundscheck)), A, convert(T,x)::T, i1, i2, I...))

# These are redundant with the abstract fallbacks but needed for bootstrap
function setindex!(A::Array, x, I::AbstractVector{Int})
@_propagate_inbounds_meta
I′ = unalias(A, I)
for i in I′
A[i] = x
end
return A
end
function setindex!(A::Array, X::AbstractArray, I::AbstractVector{Int})
@_propagate_inbounds_meta
@boundscheck setindex_shape_check(X, length(I))
X′ = unalias(A, X)
I′ = unalias(A, I)
count = 1
for i in I′
@inbounds x = X′[count]
A[i] = x
count += 1
end
return A
end

# Faster contiguous setindex! with copyto!
function setindex!(A::Array{T}, X::Array{T}, I::UnitRange{Int}) where T
@_inline_meta
@boundscheck checkbounds(A, I)
lI = length(I)
@boundscheck setindex_shape_check(X, lI)
if lI > 0
unsafe_copyto!(A, first(I), X, 1, lI)
end
return A
end
function setindex!(A::Array{T}, X::Array{T}, c::Colon) where T
@_inline_meta
lI = length(A)
@boundscheck setindex_shape_check(X, lI)
if lI > 0
unsafe_copyto!(A, 1, X, 1, lI)
end
return A
end

setindex!(A::Array, x::Number, ::Colon) = fill!(A, x)
setindex!(A::Array{T, N}, x::Number, ::Vararg{Colon, N}) where {T, N} = fill!(A, x)

Expand Down
39 changes: 0 additions & 39 deletions base/bitarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -657,9 +657,6 @@ indexoffset(::Colon) = 0
fill_chunks!(B.chunks, y, f0, l0)
return B
end
@propagate_inbounds function setindex!(B::BitArray, X::AbstractArray, J0::Union{Colon,UnitRange{Int}})
_setindex!(IndexStyle(B), B, X, to_indices(B, (J0,))[1])
end

# logical indexing

Expand All @@ -685,42 +682,6 @@ function _unsafe_setindex!(B::BitArray, x, I::BitArray)
return B
end

# Assigning an array of bools is more complicated, but we can still do some
# work on chunks by combining X and I 64 bits at a time to improve perf by ~40%
@inline function setindex!(B::BitArray, X::AbstractArray, I::BitArray)
@boundscheck checkbounds(B, I)
_unsafe_setindex!(B, X, I)
end
function _unsafe_setindex!(B::BitArray, X::AbstractArray, I::BitArray)
Bc = B.chunks
Ic = I.chunks
length(Bc) == length(Ic) || throw_boundserror(B, I)
lc = length(Bc)
lx = length(X)
last_chunk_len = _mod64(length(B)-1)+1

c = 1
for i = 1:lc
@inbounds Imsk = Ic[i]
@inbounds C = Bc[i]
u = UInt64(1)
for j = 1:(i < lc ? 64 : last_chunk_len)
if Imsk & u != 0
lx < c && throw_setindex_mismatch(X, c)
@inbounds x = convert(Bool, X[c])
C = ifelse(x, C | u, C & ~u)
c += 1
end
u <<= 1
end
@inbounds Bc[i] = C
end
if length(X) != c-1
throw_setindex_mismatch(X, c-1)
end
return B
end

## Dequeue functionality ##

function push!(B::BitVector, item)
Expand Down
8 changes: 6 additions & 2 deletions base/bitset.jl
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,16 @@ end
@inline function _growend0!(b::Bits, nchunks::Int)
len = length(b)
_growend!(b, nchunks)
@inbounds b[len+1:end] = CHK0 # resize! gives dirty memory
for i in len+1:length(b)
@inbounds b[i] = CHK0 # resize! gives dirty memory
end
end

@inline function _growbeg0!(b::Bits, nchunks::Int)
_growbeg!(b, nchunks)
@inbounds b[1:nchunks] = CHK0
for i in 1:nchunks
@inbounds b[i] = CHK0
end
end

function _matched_map!(f, s1::BitSet, s2::BitSet)
Expand Down
5 changes: 4 additions & 1 deletion base/char.jl
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,10 @@ widen(::Type{Char}) = Char

print(io::IO, c::Char) = (write(io, c); nothing)

const hex_chars = UInt8['0':'9';'a':'z']
const hex_chars = [0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38, 0x39,
0x61, 0x62, 0x63, 0x64, 0x65, 0x66, 0x67, 0x68, 0x69, 0x6a,
0x6b, 0x6c, 0x6d, 0x6e, 0x6f, 0x70, 0x71, 0x72, 0x73, 0x74,
0x75, 0x76, 0x77, 0x78, 0x79, 0x7a]

function show(io::IO, c::Char)
if c <= '\\'
Expand Down
18 changes: 18 additions & 0 deletions base/deprecated.jl
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,24 @@ function _depwarn_for_trailing_indices(t::Tuple)
true
end

# issue #24368: nonscalar indexed assignment of many values to many locations
function deprecate_nonscalar_indexed_assignment!(A::AbstractArray, X::AbstractArray, I...)
J = to_indices(A, I)
shape = Base.index_shape(J...)
if shape == axes(X) || length(X) == prod(length, shape) <= 1
depwarn("using `A[I...] = X` to implicitly broadcast the elements of `X` to many locations in `A` is deprecated. Use `A[I...] .= X` to explicitly opt-in to broadcasting.", :setindex!)
A[J...] .= X
else
depwarn("using `A[I...] = X` to implicitly broadcast the elements of `X` to many locations in `A` is deprecated. Use `A[I...] .= reshape(X, axes(view(A, I...)))` to explicitly opt-in to broadcasting.", :setindex!)
A[J...] .= reshape(X, shape)
end
return A
end
_unsafe_setindex!(::IndexStyle, A::AbstractArray, X::AbstractArray, I::Union{Real,AbstractArray}...) = deprecate_nonscalar_indexed_assignment!(A, X, I...)
setindex!(B::BitArray, X::AbstractArray, J0::Union{Colon,UnitRange{Int}}) = deprecate_nonscalar_indexed_assignment!(B, X, J0)
setindex!(B::BitArray, X::AbstractArray, I0::Union{Colon,UnitRange{Int}}, I::Union{Int,UnitRange{Int},Colon}...) = deprecate_nonscalar_indexed_assignment!(B, X, I0, I...)
setindex!(B::BitArray, X::AbstractArray, I::BitArray) = deprecate_nonscalar_indexed_assignment!(B, X, I)

# issue #22791
@deprecate select partialsort
@deprecate select! partialsort!
Expand Down
Loading