Skip to content

Commit

Permalink
Allow 2-arg splice! on arbitrary index iterables (#34524)
Browse files Browse the repository at this point in the history
We have deleteat!(vec::Vector, itr), but it returns the modified vector.
If you instead want to obtain the delted elements, there is
splice!(::Vector, ::UnitRange) as a special case of the three-argument
version that inserts additional elements where the old ones were
deleted. However, these is no two argument splice! for arbitrary
iterables, so probably the best way to write that currently is:

```
inds = collect(itr)
vals = A[inds]
deleteat!(A, inds)
vals
```

which is both less efficient and more verbose than ideal. I'm wondering
if perhaps deleteat! should have been returning the deleted elements,
but that's not a change we can make in 1.x. Instead, I'm proposing
here to extend the 2 argument (but not the 3 argument for obvious reasons)
variant of splice! to arbitrary iterables.
  • Loading branch information
Keno authored Feb 26, 2020
1 parent c1059e7 commit c2cd601
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 1 deletion.
9 changes: 8 additions & 1 deletion base/array.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1273,12 +1273,16 @@ Stacktrace:
deleteat!(a::Vector, inds) = _deleteat!(a, inds)
deleteat!(a::Vector, inds::AbstractVector) = _deleteat!(a, to_indices(a, (inds,))[1])

function _deleteat!(a::Vector, inds)
struct Nowhere; end
push!(::Nowhere, _) = nothing

function _deleteat!(a::Vector, inds, dltd=Nowhere())
n = length(a)
y = iterate(inds)
y === nothing && return a
n == 0 && throw(BoundsError(a, inds))
(p, s) = y
p <= n && push!(dltd, @inbounds a[p])
q = p+1
while true
y = iterate(inds, s)
Expand All @@ -1295,6 +1299,7 @@ function _deleteat!(a::Vector, inds)
@inbounds a[p] = a[q]
p += 1; q += 1
end
push!(dltd, @inbounds a[i])
q = i+1
end
while q <= n
Expand Down Expand Up @@ -1444,6 +1449,8 @@ function splice!(a::Vector, r::UnitRange{<:Integer}, ins=_default_splice)
return v
end

splice!(a::Vector, inds) = (dltds = eltype(a)[]; _deleteat!(a, inds, dltds); dltds)

function empty!(a::Vector)
_deleteend!(a, length(a))
return a
Expand Down
1 change: 1 addition & 0 deletions test/arrayops.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1342,6 +1342,7 @@ end
@test a == [acopy[1:(first(idx)-1)]; repl; acopy[(last(idx)+1):end]]
end
end
@test splice!([4,3,2,1], [2, 4]) == [3, 1]
end

@testset "filter!" begin
Expand Down

2 comments on commit c2cd601

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Executing the daily benchmark build, I will reply here when finished:

@nanosoldier runbenchmarks(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

Please sign in to comment.