Skip to content
18 changes: 14 additions & 4 deletions base/array.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1462,12 +1462,22 @@ julia> deleteat!([6, 5, 4, 3, 2, 1], 2)
1
```
"""
deleteat!(a::Vector, i::Integer) = (_deleteat!(a, i, 1); a)
function deleteat!(a::Vector, i::Integer)
i isa Bool && depwarn("passing Bool as an index is deprecated", :deleteat!)
_deleteat!(a, i, 1)
return a
end

function deleteat!(a::Vector, r::AbstractUnitRange{<:Integer})
n = length(a)
isempty(r) || _deleteat!(a, first(r), length(r))
return a
if eltype(r) === Bool
return invoke(deleteat!, Tuple{Vector, AbstractVector{Bool}}, a, r)
else
n = length(a)
f = first(r)
f isa Bool && depwarn("passing Bool as an index is deprecated", :deleteat!)
isempty(r) || _deleteat!(a, f, length(r))
return a
end
end

"""
Expand Down
54 changes: 52 additions & 2 deletions base/bitarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -947,6 +947,7 @@ function _deleteat!(B::BitVector, i::Int)
end

function deleteat!(B::BitVector, i::Integer)
i isa Bool && depwarn("passing Bool as an index is deprecated", :deleteat!)
i = Int(i)
n = length(B)
1 <= i <= n || throw(BoundsError(B, i))
Expand Down Expand Up @@ -987,25 +988,68 @@ function deleteat!(B::BitVector, inds)

(p, s) = y
checkbounds(B, p)
p isa Bool && throw(ArgumentError("invalid index $p of type Bool"))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can throw an error here (and not just warn about deprecation), because earlier it already threw an error as a side effect of lack of conversion of p to Int where needed later. (same below)

The current behavior is:

julia> deleteat!(falses(5), Any[true])
ERROR: MethodError: no method matching copy_chunks!(::Vector{UInt64}, ::Bool, ::Vector{UInt64}, ::Int64, ::Int64)

julia> deleteat!(falses(5), Any[1, true])
ERROR: ArgumentError: indices must be unique and sorted

Now in both cases we will throw throw(ArgumentError("invalid index $p of type Bool"))

q = p+1
new_l -= 1
y = iterate(inds, s)
while y !== nothing
(i, s) = y
if !(q <= i <= n)
i isa Bool && throw(ArgumentError("invalid index $i of type Bool"))
i < q && throw(ArgumentError("indices must be unique and sorted"))
throw(BoundsError(B, i))
end
new_l -= 1
if i > q
copy_chunks!(Bc, p, Bc, Int(q), Int(i-q))
copy_chunks!(Bc, Int(p), Bc, Int(q), Int(i-q))
p += i-q
end
q = i+1
y = iterate(inds, s)
end

q <= n && copy_chunks!(Bc, p, Bc, Int(q), Int(n-q+1))
q <= n && copy_chunks!(Bc, Int(p), Bc, Int(q), Int(n-q+1))

delta_k = num_bit_chunks(new_l) - length(Bc)
delta_k < 0 && _deleteend!(Bc, -delta_k)

B.len = new_l

if new_l > 0
Bc[end] &= _msk_end(new_l)
end

return B
end

function deleteat!(B::BitVector, inds::AbstractVector{Bool})
length(inds) == length(B) || throw(BoundsError(B, inds))

n = new_l = length(B)
y = findfirst(inds)
y === nothing && return B

Bc = B.chunks

p = y
s = y + 1
checkbounds(B, p)
q = p + 1
new_l -= 1
y = findnext(inds, s)
while y !== nothing
i = y
s = y + 1
new_l -= 1
if i > q
copy_chunks!(Bc, Int(p), Bc, Int(q), Int(i-q))
p += i - q
end
q = i + 1
y = findnext(inds, s)
end

q <= n && copy_chunks!(Bc, Int(p), Bc, Int(q), Int(n - q + 1))

delta_k = num_bit_chunks(new_l) - length(Bc)
delta_k < 0 && _deleteend!(Bc, -delta_k)
Expand All @@ -1020,6 +1064,10 @@ function deleteat!(B::BitVector, inds)
end

function splice!(B::BitVector, i::Integer)
# TODO: after deprecation remove the four lines below
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have also added the checks for Bool to splice! as they are done in non-BitVector case:

julia> splice!([1,2,3], true)
ERROR: ArgumentError: invalid index: true of type Bool

julia> splice!([1,2,3], true, [4,5])
ERROR: ArgumentError: invalid index: true of type Bool

# as v = B[i] is enough to do both bounds checking
# and Bool check then just pass Int(i) to _deleteat!
i isa Bool && depwarn("passing Bool as an index is deprecated", :splice!)
i = Int(i)
n = length(B)
1 <= i <= n || throw(BoundsError(B, i))
Expand All @@ -1032,8 +1080,10 @@ end
const _default_bit_splice = BitVector()

function splice!(B::BitVector, r::Union{AbstractUnitRange{Int}, Integer}, ins::AbstractArray = _default_bit_splice)
r isa Bool && depwarn("passing Bool as an index is deprecated", :splice!)
_splice_int!(B, isa(r, AbstractUnitRange{Int}) ? r : Int(r), ins)
end

function _splice_int!(B::BitVector, r, ins)
n = length(B)
i_f, i_l = first(r), last(r)
Expand Down
37 changes: 37 additions & 0 deletions test/bitarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1711,3 +1711,40 @@ end
@test typeof([a a ;;; a a]) <: BitArray
@test typeof([a a ;;; [a a]]) <: BitArray
end

@testset "deleteat! additional tests" begin
for v in ([1, 2, 3], [true, true, true], trues(3))
@test_throws BoundsError deleteat!(v, true:true)
end

for v in ([1], [true], trues(1))
@test length(deleteat!(v, false:false)) == 1
@test isempty(deleteat!(v, true:true))
end

x = trues(3)
x[3] = false
@test deleteat!(x, [UInt8(2)]) == [true, false]
@test_throws ArgumentError deleteat!(x, Any[true])
@test_throws ArgumentError deleteat!(x, Any[1, true])
@test_throws ArgumentError deleteat!(x, Any[2, 1])
@test_throws BoundsError deleteat!(x, Any[4])
@test_throws BoundsError deleteat!(x, Any[2, 4])

function test_equivalence(n::Int)
x1 = rand(Bool, n)
x2 = BitVector(x1)
inds1 = rand(Bool, n)
inds2 = BitVector(inds1)
return deleteat!(copy(x1), findall(inds1)) ==
deleteat!(copy(x1), inds1) ==
deleteat!(copy(x2), inds1) ==
deleteat!(copy(x1), inds2) ==
deleteat!(copy(x2), inds2)
end

Random.seed!(1234)
for n in 1:20, _ in 1:100
@test test_equivalence(n)
end
end