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

Setting OffsetArray(::StaticArray) falling back to Vector #153

Open
marius311 opened this issue Feb 10, 2021 · 3 comments
Open

Setting OffsetArray(::StaticArray) falling back to Vector #153

marius311 opened this issue Feb 10, 2021 · 3 comments

Comments

@marius311
Copy link

@set! is awesome for StaticArrays,

julia> x = @SVector(zeros(Int,3))
3-element SVector{3, Int64} with indices SOneTo(3):
 0
 0
 0

julia> @set x[1] = 1
3-element SVector{3, Int64} with indices SOneTo(3):
 1
 0
 0

but if the array if OffsetArray(::StaticArray), it seems to fall back to OffsetArray(::Array).

julia> x = OffsetArray(@SVector(zeros(Int,3)),-1)
3-element OffsetArray(::SVector{3, Int64}, 0:2) with eltype Int64 with indices 0:2:
 0
 0
 0

julia> @set x[1] = 1
3-element OffsetArray(::Vector{Int64}, 0:2) with eltype Int64 with indices 0:2:
 0
 1
 0

Is there some special-case I could define in my session to handle this? Could this be fixed generally?

Julia 1.6 / Setfield v0.7.0 / StaticArrays v1.0.1 / OffsetArrays v1.5.3

@jw3126
Copy link
Owner

jw3126 commented Feb 11, 2021

Great issue! Ideally @set x[...] would just call Base.setindex(x, ...). However Base.setindex is only defined for very few containers. And we can't increase coverage here without piracy. The best way to solve this would be

@marius311
Copy link
Author

Do a PR to OffsetArrays, that overloads setindex(::OffsetArray, ...)

I actually tried defining this locally, but Base.setindex(::OffsetArray) didnt work (if thats what you meant), however defining Setfield.setindex did work (and I suppose wouldn't be considered piracy?) Here's what I had for reference:

@inline function Setfield.setindex(A::OffsetVector{T,S}, val, i::Int) where {T,S<:StaticArray}
    @boundscheck checkbounds(A, i)
    OffsetVector{T,S}(setindex(parent(A), val, OffsetArrays.parentindex(Base.axes1(A), i)), A.offsets)
end

Anyway, you certainly know better than me, but thought I'd mention that.

@jw3126
Copy link
Owner

jw3126 commented Feb 11, 2021

Yes it is fine to do that. I will only break, if the issue is fixed properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants