Vexatos / CircularArrays.jl

Multi-dimensional arrays with fixed size and circular indexing.
MIT License
37 stars 12 forks source link

Missing push! implementation #21

Closed juliohm closed 6 months ago

juliohm commented 12 months ago
julia> using CircularArrays

julia> c = CircularVector([1,2,3])
3-element CircularVector(::Vector{Int64}):
 1
 2
 3

julia> push!(c, 4)
ERROR: MethodError: no method matching resize!(::CircularVector{Int64, Vector{Int64}}, ::Int64)

Closest candidates are:
  resize!(::Vector, ::Integer)
   @ Base array.jl:1246
  resize!(::BitVector, ::Integer)
   @ Base bitarray.jl:814
  resize!(::SparseArrays.ReadOnly, ::Any)
   @ SparseArrays ~/.julia/juliaup/julia-1.9.3+0.x64.linux.gnu/share/julia/stdlib/v1.9/SparseArrays/src/readonly.jl:33
  ...

Stacktrace:
 [1] _append!(a::CircularVector{Int64, Vector{Int64}}, #unused#::Base.HasLength, iter::Tuple{Int64})
   @ Base ./array.jl:1134
 [2] append!(a::CircularVector{Int64, Vector{Int64}}, iter::Tuple{Int64})
   @ Base ./array.jl:1126
 [3] push!(a::CircularVector{Int64, Vector{Int64}}, iter::Int64)
   @ Base ./array.jl:1127
 [4] top-level scope
   @ REPL[6]:1
terasakisatoshi commented 9 months ago

@juliohm Cc @Vexatos

Here is what I did:

Add the following methods in src/CircularArrays.jl.

Ref: https://github.com/JuliaArrays/OffsetArrays.jl/blob/6090bf49114ef86d122fc131a8bdb251d19aea3b/src/OffsetArrays.jl#L604-L608

Base.resize!(A::CircularVector, nl::Integer) = (resize!(parent(A), nl); A)
Base.push!(a::CircularVector, x...) = (push!(parent(a), x...); a)
Base.pop!(a::CircularVector) = pop!(parent(a))
Base.append!(a::CircularVector, items) = (append!(parent(a), items); a)
Base.empty!(a::CircularVector) = (empty!(parent(a)); a)

Now we have

julia> using CircularArrays
julia> c = CircularVector([1,2,3])
3-element CircularVector(::Vector{Int64}):
 1
 2
 3

julia> push!(c, 4)
4-element CircularVector(::Vector{Int64}):
 1
 2
 3
 4

julia> c
4-element CircularVector(::Vector{Int64}):
 1
 2
 3
 4

julia> push!(c, 6,7,8)
7-element CircularVector(::Vector{Int64}):
 1
 2
 3
 4
 6
 7
 8
juliohm commented 9 months ago

Thank you @terasakisatoshi!

@Vexatos is this a valid approach to be merged as a PR?

Vexatos commented 9 months ago

Since I already have deleteat! and insert! as mutating methods, these should probably exist as well.

juliohm commented 9 months ago

@terasakisatoshi it would be nice to convert your comment to a PR 💯

terasakisatoshi commented 9 months ago

I found that when Base.resize!(A::CircularVector, nl::Integer) = (resize!(parent(A), nl); A) is defined, we can use push!.

Vexatos commented 6 months ago

This is now available in Version 1.4.0.