Skip to content

set(arr, reverse, arr2) can change size - #213

Open
jariji wants to merge 4 commits into
JuliaObjects:masterfrom
jariji:patch-1
Open

jariji wants to merge 4 commits into
JuliaObjects:masterfrom
jariji:patch-1

Conversation

@jariji

@jariji jariji commented Sep 13, 2025

Copy link
Copy Markdown

Allow reversed objects to change size. E.g.

julia> @modify([10,20,30]) do xs
           xs[2:3]
       end
2-element Vector{Int64}:
 20
 30

Resolves #212.

@jariji jariji left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I'm not sure why set isn't hitting the method in the extension. If I define those methods directly in the repl the right one is selected and the test passes.

julia> using AxisKeys,Accessors, Test

julia> function Accessors.set(x::AbstractVector, ::typeof(reverse), v)
           res = similar(x, eltype(v))
           res .= v
           reverse!(res)
           res
       end
       function Accessors.set(x::AbstractVector, ::typeof(reverse), v::AbstractVector)
           res = similar(x, eltype(v), size(v))
           res .= v
           reverse!(res)
           res
       end
       function Accessors.set(x::AxisKeys.KeyedArray, ::typeof(reverse), v::AbstractVector)
           res = similar(x, eltype(v))
           res .= v
           reverse!(res)
           res
       end
       let
           C = KeyedArray([1,2,3], x=[:a, :b, :c])
           @test (@set reverse(C) = [5,6,7])::KeyedArray == KeyedArray([7,6,5], x=[:a, :b, :c])
       end
Test Passed

@aplavin

aplavin commented Sep 13, 2025 •

Copy link
Copy Markdown
Member

Thanks for trying it out @jariji!

It's not just KeyedArrays that need special handling after this PR. For example, StaticArrays now lose their "staticness" completely: set(SVector(1,2,3), reverse, 10:12) now returns a plain Vector without a static length.
Maybe there are even more cases like that... But even with 2 major "special cases", I'm personally not fully convinced this is the way to go.

@aplavin

aplavin commented Sep 13, 2025 •

Copy link
Copy Markdown
Member

But the extension issue itself is simple: you define a new set function that doesn't overload Accessors.set.

@jariji jariji changed the title set(arr, reverse, arr) can change size set(arr, reverse, arr2) can change size Sep 13, 2025

This branch has not been deployed

No deployments
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

Successfully merging this pull request may close these issues.

DimensionMismatch after slicing

2 participants