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

Functional non-mutating methods. #46453

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open
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
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ New library functions
* New function `stack(x)` which generalises `reduce(hcat, x::Vector{<:Vector})` to any dimensionality,
and allows any iterators of iterators. Method `stack(f, x)` generalises `mapreduce(f, hcat, x)` and
is efficient. ([#43334])
* New functions `delete`, `deleteat`, and `insert` provide non-mutating counterparts to `delete!`, `deleteat!`, and `insert!` ([#46453]).
* New macro `@allocations` which is similar to `@allocated` except reporting the total number of allocations
rather than the total size of memory allocated. ([#47367])

Expand All @@ -95,6 +96,7 @@ Library changes
* `@time` now separates out % time spent recompiling invalidated methods ([#45015]).
* `eachslice` now works over multiple dimensions; `eachslice`, `eachrow` and `eachcol` return
a `Slices` object, which allows dispatching to provide more efficient methods ([#32310]).
* The non-mutationg `Base.setindex` function now has `AbstractDict` support ([#46453]).
Copy link
Contributor

Choose a reason for hiding this comment

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

mutationg

* `@kwdef` is now exported and added to the public API ([#46273])
* An issue with order of operations in `fld1` is now fixed ([#28973]).
* Sorting is now always stable by default as `QuickSort` was stabilized in ([#45222]).
Expand Down
16 changes: 16 additions & 0 deletions base/abstractdict.jl
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,22 @@ end
# that we need to avoid dispatch loops if setindex!(t,v,k) is not defined.)
getindex(t::AbstractDict, k1, k2, ks...) = getindex(t, tuple(k1,k2,ks...))
setindex!(t::AbstractDict, v, k1, k2, ks...) = setindex!(t, v, tuple(k1,k2,ks...))
function setindex(src::AbstractDict{K,V}, val::V, key::K) where {K,V}
dst = copy(src)
dst[key] = val
return dst
end
function setindex(src::AbstractDict{K,V}, val, key) where {K,V}
dst = empty(src, promote_type(K,typeof(key)), promote_type(V,typeof(val)))
if haslength(src)
sizehint!(dst, length(src))
end
for (k,v) in src
dst[k] = v
end
dst[key] = val
return dst
end

get!(t::AbstractDict, key, default) = get!(() -> default, t, key)
function get!(default::Callable, t::AbstractDict{K,V}, key) where K where V
Expand Down
7 changes: 7 additions & 0 deletions base/accumulate.jl
Original file line number Diff line number Diff line change
Expand Up @@ -434,3 +434,10 @@ function _accumulate1!(op, B, v1, A::AbstractVector, dim::Integer)
end
return B
end

function setindex(t::Tuple, v, inds::AbstractVector{<:Integer})
Base.@_propagate_inbounds_meta
foldl(ntuple(identity, length(inds)); init=t) do acc, i
Base.setindex(acc, v[i], inds[i])
end
end
226 changes: 226 additions & 0 deletions base/array.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1007,6 +1007,64 @@ function setindex!(A::Array{T}, X::Array{T}, c::Colon) where T
return A
end

"""
setindex(collection, value, key...)

Create a new collection with the element/elements of the location specified by `key`
replaced with `value`(s).

Note that this may be particularly expensive when `collection` is large and whose storage
cannot be shared between seperate instances.

# Implementation

`setindex(collection, value, key...)` must have the property such that

```julia
y1 = setindex(x, value, key...)

y2 = copy′(x)
@assert convert.(eltype(y2), x) == y2

y2[key...] = value
@assert convert.(eltype(y2), y1) == y2
```

with a suitable definition of `copy′` such that `y2[key...] = value` succeeds.

`setindex` should support more combinations of arguments by widening collection
type as required.
"""
function setindex end

function setindex(x::AbstractArray, v, i...)
@_propagate_inbounds_meta
inds = to_indices(x, i)
if inds isa Tuple{Vararg{Integer}}
T = promote_type(eltype(x), typeof(v))
else
T = promote_type(eltype(x), eltype(v))
end
y = similar(x, T)
copy!(y, x)
y[inds...] = v
return y
end
function setindex(t::Tuple, v, inds::AbstractUnitRange{<:Integer})
Tokazama marked this conversation as resolved.
Show resolved Hide resolved
@boundscheck checkindex(Bool, eachindex(t), inds) || throw(BoundsError(t, inds))
@boundscheck setindex_shape_check(v, length(inds))
start = first(inds)
stop = last(inds)
offset = start - firstindex(v)
ntuple(Val{nfields(t)}()) do i
if i < start || i > stop
getfield(t, i)
else
v[i - offset]
end
end
end

# efficiently grow an array

_growbeg!(a::Vector, delta::Integer) =
Expand Down Expand Up @@ -1145,6 +1203,63 @@ function _append!(a, ::IteratorSize, iter)
a
end

"""
insert(a::AbstractVector, index::Integer, item)

Returns a copy of `a` with `item` inserted at `index`.

See also: [`insert!`](@ref), [`deleteat`](@ref), [`delete`](@ref)

# Examples
```jldoctest
julia> A = Any[1, 2, 3]
3-element Vector{Any}:
1
2
3

julia> insert(A, 3, "here") == [1, 2, "here", 3]
true

julia> A == [1, 2, 3]
true
````
"""
function insert(src::AbstractVector, index::Integer, item)
src_idx = firstindex(src)
src_end = lastindex(src)
if index == (src_end + 1)
dst = similar(src, promote_type(eltype(src), typeof(item)), length(src) + 1)
for dst_idx in eachindex(dst)
@inbounds if isassigned(src, src_idx)
@inbounds dst[dst_idx] = src[src_idx]
end
src_idx += 1
end
@inbounds dst[end] = item
else
@boundscheck (src_end < index || index < src_idx) && throw(BoundsError(src, index))
dst = similar(src, promote_type(eltype(src), typeof(item)), length(src) + 1)
dst_idx = firstindex(dst)
while src_idx < index
@inbounds if isassigned(src, src_idx)
@inbounds dst[dst_idx] = src[src_idx]
end
dst_idx += 1
src_idx += 1
end
setindex!(dst, item, dst_idx)
dst_idx += 1
for i in src_idx:src_end
@inbounds if isassigned(src, i)
@inbounds dst[dst_idx] = src[i]
end
dst_idx += 1
end
end
dst
end

"""
prepend!(a::Vector, collections...) -> collection

Expand Down Expand Up @@ -1633,6 +1748,117 @@ function deleteat!(a::Vector, inds::AbstractVector{Bool})
return a
end

"""
deleteat(a::Vector, i::Integer)

Remove the item at the given `i` and return the modified `a`. Subsequent items
are shifted to fill the resulting gap.

See also: [`deleteat!`](@ref), [`delete`](@ref), [`insert`](@ref)

# Examples
```jldoctest
julia> x = [6, 5, 4]
3-element Vector{Int64}:
6
5
4

julia> deleteat(x, 2) == [6, 4]
true

julia> deleteat(x, [2, 3]) == [6]
true

julia> x == [6, 5, 4]
true
```
"""
function deleteat(src::AbstractVector, i::Integer)
Copy link
Contributor

Choose a reason for hiding this comment

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

deleteat(src::AbstractVector, i) = deleteat!(copy(src), i)

?
Instead of all these methods.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I see - it may work with setindex-able, but not resizeable arrays as of now...
Unfortunate that so much duplication is needed.

Copy link
Contributor Author

@Tokazama Tokazama Mar 2, 2023

Choose a reason for hiding this comment

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

Yeah, it's a bit messy. I thought about putting # TODO refactor when immutable arrays are in base but I don't think we know how that's going to play out still. Also, I use "TODO" as a keyword in searches for my code bases, so I try not to pollute community projects with it unless it's clearly helpful for everyone.

src_idx = firstindex(src)
src_end = lastindex(src)
src_idx <= i <= src_end || throw(BoundsError(src, i))
dst = similar(src, length(src) - 1)
dst_idx = firstindex(dst)
while src_idx <= src_end
if src_idx != i
@inbounds isassigned(src, src_idx) && setindex!(dst, src[src_idx], dst_idx)
dst_idx += 1
end
src_idx += 1
end
dst
end
function deleteat(src::AbstractVector, inds::AbstractVector{Bool})
n = length(src)
length(inds) == n || throw(BoundsError(src, inds))
dst = similar(src, n - count(inds))
dst_idx = firstindex(dst)
src_idx = firstindex(src)
for index in inds
if !index
@inbounds isassigned(src, src_idx) && setindex!(dst, src[src_idx], dst_idx)
dst_idx += 1
end
src_idx += 1
end
dst
end
function deleteat(src::AbstractVector, inds::AbstractUnitRange{<:Integer})
srcinds = eachindex(src)
N = length(srcinds)
dst = similar(src, N - length(inds))
src_idx = first(srcinds)
dst_idx = firstindex(dst)
while src_idx < first(inds)
@inbounds isassigned(src, src_idx) && setindex!(dst, src[src_idx], dst_idx)
src_idx += 1
dst_idx += 1
end
src_idx = last(inds) + 1
while src_idx <= N
@inbounds isassigned(src, src_idx) && setindex!(dst, src[src_idx], dst_idx)
src_idx += 1
dst_idx += 1
end
dst
end
function deleteat(src::AbstractVector, inds)
itr = iterate(inds)
if itr === nothing
copy(src)
else
len = length(src) - length(inds)
# `length(inds) > length(src)` then `inds` has repeated or out of bounds indices
len > 0 || throw(BoundsError(src, inds))
index, state = itr
dst = similar(src, len)
dst_idx = firstindex(dst)
src_idx = firstindex(src)
src_end = lastindex(src)
src_idx > index && throw(BoundsError(src, inds))
while true
(src_end < index || index < src_idx) && throw(BoundsError(src, inds))
while src_idx < index
@inbounds isassigned(src, src_idx) && setindex!(dst, src[src_idx], dst_idx)
dst_idx += 1
src_idx += 1
end
src_idx += 1
itr = iterate(inds, state)
itr === nothing && break
itr[1] <= index && throw(ArgumentError("indices must be unique and sorted"))
index, state = itr
end
while src_idx <= src_end
@inbounds isassigned(src, src_idx) && setindex!(dst, src[src_idx], dst_idx)
dst_idx += 1
src_idx += 1
end
dst
end
end

const _default_splice = []

"""
Expand Down
58 changes: 57 additions & 1 deletion base/dict.jl
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,6 @@ function setindex!(h::Dict{K,Any}, v, key::K) where K
return h
end


"""
get!(collection, key, default)

Expand Down Expand Up @@ -669,6 +668,30 @@ function delete!(h::Dict, key)
return h
end

"""
delete(collection, key)

Create and return a new collection containing all the elements from `collection` except for
the mapping corresponding to `key`.

Note that this may be particularly expensive when `collection` is large and whose storage
cannot be shared between seperate instances.

See also: [`delete!`](@ref).

# Examples
```jldoctest
julia> d = Dict("a"=>1, "b"=>2);

julia> delete(d, "b") == Dict("a"=>1)
true

julia> d == Dict("a"=>1, "b"=>2)
true
```
"""
delete(collection, k) = delete!(copy(collection), k)

function skip_deleted(h::Dict, i)
L = length(h.slots)
for i = i:L
Expand Down Expand Up @@ -821,6 +844,39 @@ function get(default::Callable, dict::ImmutableDict, key)
return default()
end

function delete(d::ImmutableDict{K,V}, key) where {K,V}
if isdefined(d, :parent)
if isequal(d.key, key)
d.parent
else
ImmutableDict{K,V}(delete(d.parent, key), d.key, d.value)
end
else
d
end
end
Comment on lines +847 to +857
Copy link
Member

Choose a reason for hiding this comment

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

This implementation is not quite correct since I can current;y construct an ImmutableDict where there are multiple entries for key each shadowed.

Copy link
Member

@vtjnash vtjnash Sep 11, 2023

Choose a reason for hiding this comment

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

I suspect delete would be more easily done with adding a new key as a tombstone (with the same key, but val being #undef), rather than a bulk copy like this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suspect delete would be more easily done with adding a new key as a tombstone (with the same key, but val being #undef), rather than a bulk copy like this.

I really like that idea, but I think we would need to have a new type since we can't rely on #undef for bits types.


function setindex(d::ImmutableDict, v, k)
if isdefined(d, :parent)
if isequal(d.key, k)
d0 = d.parent
v0 = v
k0 = k
else
d0 = setindex(d.parent, v, k)
v0 = d.value
k0 = d.key
end
else
d0 = d
v0 = v
k0 = k
end
K = promote_type(keytype(d0), typeof(k0))
V = promote_type(valtype(d0), typeof(v0))
ImmutableDict{K,V}(d0, k0, v0)
end

# this actually defines reverse iteration (e.g. it should not be used for merge/copy/filter type operations)
function iterate(d::ImmutableDict{K,V}, t=d) where {K, V}
!isdefined(t, :parent) && return nothing
Expand Down
3 changes: 3 additions & 0 deletions base/exports.jl
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,7 @@ export
# dequeues
append!,
insert!,
insert,
pop!,
popat!,
prepend!,
Expand All @@ -512,7 +513,9 @@ export
count!,
count,
delete!,
delete,
deleteat!,
deleteat,
keepat!,
eltype,
empty!,
Expand Down
Loading