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

Leaves optic #14

Open
cscherrer opened this issue Dec 11, 2020 · 6 comments
Open

Leaves optic #14

cscherrer opened this issue Dec 11, 2020 · 6 comments

Comments

@cscherrer
Copy link

@jw3126 I think this is working well now. What do you think of moving Leaves into Accessors? It's very fast, and seems it could be useful for others.

Most of the code is here. Biggest concern I could see is that it depends on GeneralizedGenerated. There may be a way to drop that dependency, but it would take some fiddling.

It's no trouble to me either way, and I won't be offended if you don't think it's a good fit. Just seems more natural to me to have it in Accessors.

@jw3126
Copy link
Member

jw3126 commented Dec 11, 2020

Nice! Accessing the leaves of a tree is certainly an important task. As I understand, this assumes that the tree structure is encoded using Tuples and NamedTuples. There are many other possible choices, however. What if I have a tree like this:

["some", ["tree", [["encoded", "using"], "vectors"]

Is there a way to let the user supply the tree structure?

@cscherrer
Copy link
Author

Hmm, good question. It's not clear to me in this case even what we'd want the result to look like. Julia infers the type as Array{Any, 1}, so that doesn't help.

OTOH, if we instead had some nested structs, that should be manageable. StructArrays has some nice tricks for recasting a struct as a named tuple, and after that it reduces to the previous case.

My interest here was to just work with the part that can be manipulated very quickly. So one approach could be to use this as an "outer optic" and pass to something else for each resolved component.

Then I guess part of the question is whether the current results should be considered "leaves". Maybe TupleLeaves or something is a better name?

@jw3126
Copy link
Member

jw3126 commented Dec 11, 2020

Hmm, good question. It's not clear to me in this case even what we'd want the result to look like. Julia infers the type as Array{Any, 1}, so that doesn't help.

I would expect bad performance in this case, but the semantics would be:

tree = ["some", ["tree", [["encoded", "using"], "vectors"]
modify(uppercase, tree, Leaves(...)) == ["SOME", ["TREE", [["ENCODED", "USING"], "VECTORS"]

My point was that there are many ways trees are encoded and instead of only supporting one somewhat arbitrary way (descent into Tuples and NamedTuples) I would prefer if Accessors.jl could be helpful for all these cases.

My interest here was to just work with the part that can be manipulated very quickly. So one approach could be to use this as an "outer optic" and pass to something else for each resolved component.

I think Recursive goes in this direction. Let's hope inference can handle it 😄

@jw3126
Copy link
Member

jw3126 commented Dec 11, 2020

using Accessors
x = (a = [1, 2], b = (c = ([3, 4], [5, 6]), d = [7, 8]));
optic = Recursive(x -> x isa Union{Tuple, NamedTuple}, Elements())
@test modify(sum, x, optic, ) === (a = 3, b = (c = (7, 11), d = 15))

But sadly

@code_warntype modify(sum, x, optic, )

Variables
  #self#::Core.Compiler.Const(Accessors.modify, false)
  f::Core.Compiler.Const(sum, false)
  obj::NamedTuple{(:a, :b),Tuple{Array{Int64,1},NamedTuple{(:c, :d),Tuple{Tuple{Array{Int64,1},Array{Int64,1}},Array{Int64,1}}}}}
  optic::Core.Compiler.Const(Recursive{var"#3#4",Elements}(var"#3#4"(), Elements()), false)

Body::NamedTuple{(:a, :b),_A} where _A<:Tuple
1nothing%2 = Accessors.OpticStyle(optic)::Core.Compiler.Const(Accessors.ModifyBased(), false)
│   %3 = Accessors._modify(f, obj, optic, %2)::NamedTuple{(:a, :b),_A} where _A<:Tuple
└──      return %3

@cscherrer
Copy link
Author

Yeah, the reason I started down this path was that statically-typed things should have better performance than I was able to get out of Recursive. But maybe there's a way to fix that more directly, I'm not sure

@aplavin
Copy link
Collaborator

aplavin commented Apr 23, 2023

You may find RecursiveOfType recently added to AccessorsExtra helpful:

julia> x = (a = (1, 2), b = (c = ((3, 4), (5, 6)), d = (7, 8)));
julia> o = RecursiveOfType(Tuple{Vararg{Number}})

julia> @btime modify($sum, $x, $o)
  2.903 ns (0 allocations: 0 bytes)
(a = 3, b = (c = (7, 11), d = 15))

julia> @btime getall($x, $o)
  3.272 ns (0 allocations: 0 bytes)
((1, 2), (3, 4), (5, 6), (7, 8))

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

3 participants