From 30096c3a857045398b3772b596a5150c2879c744 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bogumi=C5=82=20Kami=C5=84ski?= Date: Tue, 7 Sep 2021 13:59:05 +0200 Subject: [PATCH 01/13] Improve corner cases of deleteat! --- base/bitarray.jl | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/base/bitarray.jl b/base/bitarray.jl index 6fe94df378516..37a10bad0c0ef 100644 --- a/base/bitarray.jl +++ b/base/bitarray.jl @@ -947,6 +947,7 @@ function _deleteat!(B::BitVector, i::Int) end function deleteat!(B::BitVector, i::Integer) + i isa Bool && depawrn("passing Bool as an index is deprecated", deleteat!) i = Int(i) n = length(B) 1 <= i <= n || throw(BoundsError(B, i)) @@ -992,6 +993,7 @@ function deleteat!(B::BitVector, inds) y = iterate(inds, s) while y !== nothing (i, s) = y + i isa Bool && depawrn("passing Bool as an index is deprecated", deleteat!) if !(q <= i <= n) i < q && throw(ArgumentError("indices must be unique and sorted")) throw(BoundsError(B, i)) @@ -1019,6 +1021,13 @@ function deleteat!(B::BitVector, inds) return B end +function deleteat!(B::BitVector, inds::AbstractVector{Bool}) + if length(B) != length(inds) + throw(ArgumentError("length of Bool indices inds must match the length of B")) + end + return deleteat!(B, findall(inds)) +end + function splice!(B::BitVector, i::Integer) i = Int(i) n = length(B) From 1b24441f55a6d80e1ba0e8db2cfa6c4d1433499e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bogumi=C5=82=20Kami=C5=84ski?= Date: Tue, 7 Sep 2021 15:23:13 +0200 Subject: [PATCH 02/13] Update array.jl --- base/array.jl | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/base/array.jl b/base/array.jl index bd9d3b8733541..882993fd6c302 100644 --- a/base/array.jl +++ b/base/array.jl @@ -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 && depawrn("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) isa Bool + return invoke(deleteat!, Tuple{Vector, AbstractVector{Bool}, a, r) + else + n = length(a) + f = first(r) + f isa Bool && depawrn("passing Bool as an index is deprecated", deleteat!) + isempty(r) || _deleteat!(a, f, length(r)) + return a + end end """ @@ -1517,6 +1527,7 @@ function _deleteat!(a::Vector, inds, dltd=Nowhere()) y = iterate(inds, s) y === nothing && break (i,s) = y + i isa Bool && depawrn("passing Bool as an index is deprecated", deleteat!) if !(q <= i <= n) if i < q throw(ArgumentError("indices must be unique and sorted")) From bf4bec02d96a8ab8ac35e72df283986efb45f28e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bogumi=C5=82=20Kami=C5=84ski?= Date: Tue, 7 Sep 2021 15:26:21 +0200 Subject: [PATCH 03/13] Update base/array.jl --- base/array.jl | 1 - 1 file changed, 1 deletion(-) diff --git a/base/array.jl b/base/array.jl index 882993fd6c302..d26c64bee299e 100644 --- a/base/array.jl +++ b/base/array.jl @@ -1527,7 +1527,6 @@ function _deleteat!(a::Vector, inds, dltd=Nowhere()) y = iterate(inds, s) y === nothing && break (i,s) = y - i isa Bool && depawrn("passing Bool as an index is deprecated", deleteat!) if !(q <= i <= n) if i < q throw(ArgumentError("indices must be unique and sorted")) From 9444ed46fc39f4c985294fd4d3d84e534c40ee83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bogumi=C5=82=20Kami=C5=84ski?= Date: Tue, 7 Sep 2021 15:34:46 +0200 Subject: [PATCH 04/13] Update bitarray.jl --- base/bitarray.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/bitarray.jl b/base/bitarray.jl index 37a10bad0c0ef..38223a59b8a1d 100644 --- a/base/bitarray.jl +++ b/base/bitarray.jl @@ -1000,7 +1000,7 @@ function deleteat!(B::BitVector, inds) 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 From e630812e1dc9c3f395b458d1c6c6407d5e07d3d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bogumi=C5=82=20Kami=C5=84ski?= Date: Tue, 7 Sep 2021 15:35:40 +0200 Subject: [PATCH 05/13] Update base/array.jl --- base/array.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/array.jl b/base/array.jl index d26c64bee299e..ee0dcf873f117 100644 --- a/base/array.jl +++ b/base/array.jl @@ -1470,7 +1470,7 @@ end function deleteat!(a::Vector, r::AbstractUnitRange{<:Integer}) if eltype(r) isa Bool - return invoke(deleteat!, Tuple{Vector, AbstractVector{Bool}, a, r) + return invoke(deleteat!, Tuple{Vector, AbstractVector{Bool}}, a, r) else n = length(a) f = first(r) From 89ef38af4d24b243bea553669841af76e529ebdf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bogumi=C5=82=20Kami=C5=84ski?= Date: Wed, 8 Sep 2021 10:46:24 +0200 Subject: [PATCH 06/13] Apply suggestions from code review Co-authored-by: Daniel Karrasch --- base/array.jl | 4 ++-- base/bitarray.jl | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/base/array.jl b/base/array.jl index ee0dcf873f117..7e9f556a9bdfa 100644 --- a/base/array.jl +++ b/base/array.jl @@ -1463,7 +1463,7 @@ julia> deleteat!([6, 5, 4, 3, 2, 1], 2) ``` """ function deleteat!(a::Vector, i::Integer) - i isa Bool && depawrn("passing Bool as an index is deprecated", deleteat!) + i isa Bool && depwarn("passing Bool as an index is deprecated", deleteat!) _deleteat!(a, i, 1) return a end @@ -1474,7 +1474,7 @@ function deleteat!(a::Vector, r::AbstractUnitRange{<:Integer}) else n = length(a) f = first(r) - f isa Bool && depawrn("passing Bool as an index is deprecated", deleteat!) + f isa Bool && depwarn("passing Bool as an index is deprecated", deleteat!) isempty(r) || _deleteat!(a, f, length(r)) return a end diff --git a/base/bitarray.jl b/base/bitarray.jl index 38223a59b8a1d..d4f198a900d7a 100644 --- a/base/bitarray.jl +++ b/base/bitarray.jl @@ -947,7 +947,7 @@ function _deleteat!(B::BitVector, i::Int) end function deleteat!(B::BitVector, i::Integer) - i isa Bool && depawrn("passing Bool as an index is deprecated", deleteat!) + 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)) @@ -993,7 +993,7 @@ function deleteat!(B::BitVector, inds) y = iterate(inds, s) while y !== nothing (i, s) = y - i isa Bool && depawrn("passing Bool as an index is deprecated", deleteat!) + i isa Bool && depwarn("passing Bool as an index is deprecated", deleteat!) if !(q <= i <= n) i < q && throw(ArgumentError("indices must be unique and sorted")) throw(BoundsError(B, i)) From 3533746a29066d8b6945e51f517e6a7e763dbba7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bogumi=C5=82=20Kami=C5=84ski?= Date: Thu, 16 Sep 2021 19:07:08 +0200 Subject: [PATCH 07/13] Update base/array.jl Co-authored-by: Jeff Bezanson --- base/array.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/array.jl b/base/array.jl index 7e9f556a9bdfa..ab975424aec4d 100644 --- a/base/array.jl +++ b/base/array.jl @@ -1469,7 +1469,7 @@ function deleteat!(a::Vector, i::Integer) end function deleteat!(a::Vector, r::AbstractUnitRange{<:Integer}) - if eltype(r) isa Bool + if eltype(r) === Bool return invoke(deleteat!, Tuple{Vector, AbstractVector{Bool}}, a, r) else n = length(a) From 3adf111c3b0d8b258fcd2cfb93468ee8472fc684 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bogumi=C5=82=20Kami=C5=84ski?= Date: Thu, 16 Sep 2021 19:44:15 +0200 Subject: [PATCH 08/13] implement faster deleteat! and initial tests --- base/bitarray.jl | 38 +++++++++++++++++++++++++++++++++++++- test/bitarray.jl | 19 +++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/base/bitarray.jl b/base/bitarray.jl index d4f198a900d7a..6f6cec4db41aa 100644 --- a/base/bitarray.jl +++ b/base/bitarray.jl @@ -1025,7 +1025,43 @@ function deleteat!(B::BitVector, inds::AbstractVector{Bool}) if length(B) != length(inds) throw(ArgumentError("length of Bool indices inds must match the length of B")) end - return deleteat!(B, findall(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, 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 splice!(B::BitVector, i::Integer) diff --git a/test/bitarray.jl b/test/bitarray.jl index b565252664876..2a50270e51f11 100644 --- a/test/bitarray.jl +++ b/test/bitarray.jl @@ -1711,3 +1711,22 @@ end @test typeof([a a ;;; a a]) <: BitArray @test typeof([a a ;;; [a a]]) <: BitArray end + +@testset "deleteat! with Bool indices" begin + 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 \ No newline at end of file From a74d9083d2f77af45d90d789463503b4dd8ea26b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bogumi=C5=82=20Kami=C5=84ski?= Date: Fri, 17 Sep 2021 00:10:01 +0200 Subject: [PATCH 09/13] add tests --- base/bitarray.jl | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/base/bitarray.jl b/base/bitarray.jl index 6f6cec4db41aa..16510adf28b64 100644 --- a/base/bitarray.jl +++ b/base/bitarray.jl @@ -1022,9 +1022,7 @@ function deleteat!(B::BitVector, inds) end function deleteat!(B::BitVector, inds::AbstractVector{Bool}) - if length(B) != length(inds) - throw(ArgumentError("length of Bool indices inds must match the length of B")) - end + length(inds) == length(B) || throw(BoundsError(a, inds)) n = new_l = length(B) y = findfirst(inds) From 5c9fbb38e776753043a6c4e13f3b3fbfad08151a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bogumi=C5=82=20Kami=C5=84ski?= Date: Fri, 17 Sep 2021 00:10:09 +0200 Subject: [PATCH 10/13] add tests --- test/bitarray.jl | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/test/bitarray.jl b/test/bitarray.jl index 2a50270e51f11..de4b8f7f659b3 100644 --- a/test/bitarray.jl +++ b/test/bitarray.jl @@ -1712,7 +1712,20 @@ end @test typeof([a a ;;; [a a]]) <: BitArray end -@testset "deleteat! with Bool indices" begin +@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] + function test_equivalence(n::Int) x1 = rand(Bool, n) x2 = BitVector(x1) From 9d9c09ecfbcfa887df87b2518d499bb7119e8e0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bogumi=C5=82=20Kami=C5=84ski?= Date: Sat, 18 Sep 2021 00:09:56 +0200 Subject: [PATCH 11/13] improve implementation in corner cases --- base/bitarray.jl | 7 ++++--- test/bitarray.jl | 5 +++++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/base/bitarray.jl b/base/bitarray.jl index 16510adf28b64..52c6142192841 100644 --- a/base/bitarray.jl +++ b/base/bitarray.jl @@ -988,13 +988,14 @@ function deleteat!(B::BitVector, inds) (p, s) = y checkbounds(B, p) + p isa Bool && throw(ArgumentError("invalid index $p of type Bool")) q = p+1 new_l -= 1 y = iterate(inds, s) while y !== nothing (i, s) = y - i isa Bool && depwarn("passing Bool as an index is deprecated", deleteat!) 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 @@ -1007,7 +1008,7 @@ function deleteat!(B::BitVector, inds) 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) @@ -1048,7 +1049,7 @@ function deleteat!(B::BitVector, inds::AbstractVector{Bool}) y = findnext(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) diff --git a/test/bitarray.jl b/test/bitarray.jl index de4b8f7f659b3..291317a1444d3 100644 --- a/test/bitarray.jl +++ b/test/bitarray.jl @@ -1725,6 +1725,11 @@ 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) From ba04859ce711fddc12772838f525386ff80e4d27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bogumi=C5=82=20Kami=C5=84ski?= Date: Sat, 18 Sep 2021 09:01:49 +0200 Subject: [PATCH 12/13] code fixes --- base/bitarray.jl | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/base/bitarray.jl b/base/bitarray.jl index 52c6142192841..ca768c5f9aedc 100644 --- a/base/bitarray.jl +++ b/base/bitarray.jl @@ -1023,7 +1023,7 @@ function deleteat!(B::BitVector, inds) end function deleteat!(B::BitVector, inds::AbstractVector{Bool}) - length(inds) == length(B) || throw(BoundsError(a, inds)) + length(inds) == length(B) || throw(BoundsError(B, inds)) n = new_l = length(B) y = findfirst(inds) @@ -1064,6 +1064,10 @@ function deleteat!(B::BitVector, inds::AbstractVector{Bool}) end function splice!(B::BitVector, i::Integer) + # TODO: after deprecation remove the four lines below + # 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)) @@ -1076,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) From bd3fbbf9ee708cdcb11d783b8d56ac89fac47a39 Mon Sep 17 00:00:00 2001 From: Jeff Bezanson Date: Tue, 21 Sep 2021 15:17:12 -0400 Subject: [PATCH 13/13] Apply suggestions from code review --- base/array.jl | 4 ++-- base/bitarray.jl | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/base/array.jl b/base/array.jl index ab975424aec4d..99abd42fa91ef 100644 --- a/base/array.jl +++ b/base/array.jl @@ -1463,7 +1463,7 @@ julia> deleteat!([6, 5, 4, 3, 2, 1], 2) ``` """ function deleteat!(a::Vector, i::Integer) - i isa Bool && depwarn("passing Bool as an index is deprecated", deleteat!) + i isa Bool && depwarn("passing Bool as an index is deprecated", :deleteat!) _deleteat!(a, i, 1) return a end @@ -1474,7 +1474,7 @@ function deleteat!(a::Vector, r::AbstractUnitRange{<:Integer}) else n = length(a) f = first(r) - f isa Bool && depwarn("passing Bool as an index is deprecated", deleteat!) + f isa Bool && depwarn("passing Bool as an index is deprecated", :deleteat!) isempty(r) || _deleteat!(a, f, length(r)) return a end diff --git a/base/bitarray.jl b/base/bitarray.jl index ca768c5f9aedc..c70ace3f8f603 100644 --- a/base/bitarray.jl +++ b/base/bitarray.jl @@ -947,7 +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 isa Bool && depwarn("passing Bool as an index is deprecated", :deleteat!) i = Int(i) n = length(B) 1 <= i <= n || throw(BoundsError(B, i)) @@ -1067,7 +1067,7 @@ function splice!(B::BitVector, i::Integer) # TODO: after deprecation remove the four lines below # 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 isa Bool && depwarn("passing Bool as an index is deprecated", :splice!) i = Int(i) n = length(B) 1 <= i <= n || throw(BoundsError(B, i)) @@ -1080,7 +1080,7 @@ 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!) + r isa Bool && depwarn("passing Bool as an index is deprecated", :splice!) _splice_int!(B, isa(r, AbstractUnitRange{Int}) ? r : Int(r), ins) end