jw3126 / Setfield.jl

Update deeply nested immutable structs.
Other
167 stars 17 forks source link

RFC: Function as a Lens #77

Closed tkf closed 5 years ago

tkf commented 5 years ago

I propose to add a new lens FunctionLens(f) with a sugar @lens f(_). By default, it only has the get method which is defined to be:

get(x, @lens f(_)) == f(x)

Unlike other lenses, FunctionLens needs set method to be defined manually.

It would be useful for manipulating objects that do not have clear indexing or property access. For example, manipulating parameterized types:

julia> obj = Vector{Int}
Array{Int64,1}

julia> @set eltype(obj) = Float32
Array{Float32,1}

julia> obj = Dict{Symbol, Dict{Int, Float64}}
Dict{Symbol,Dict{Int64,Float64}}

julia> @set keytype(valtype(obj)) = String
Dict{Symbol,Dict{String,Float64}}

A scary part of this proposal is that Base/stdlib methods on Base/stdlib types would be accumulated in this package to avoid type piracy. It could bloat otherwise a slick package. My guess/hope is that there are not too many functions with useful FunctionLens definition as otherwise Base/stdlib wouldn't have been usable.

I think the upside of having @lens f(_) is greater than the potential downside:

Some discussion points:

jw3126 commented 5 years ago

Thanks for the PR. This is a great idea and we should probably have this here! I wonder if there is a mechanism to hook into e.g. @set, @lens, ... macro and so rules like this could live in an external package. E.g. Kaleido.

I am a bit in a hurry, I will add more comments later.

cstjean commented 5 years ago

It reminds me of Common Lisp's setf, which also supported assigning to arbitrary expressions.

https://www.cs.cmu.edu/Groups/AI/html/cltl/clm/node80.html http://www.lispworks.com/documentation/HyperSpec/Body/m_defset.htm#defsetf

jw3126 commented 5 years ago

Some more comments:

julia> @set identity(2) = 3 3

julia> @macroexpand @set identity(2) = 3 quote

= /home/jan/.julia/packages/Setfield/gMRkV/src/sugar.jl:112 =

#3#lens = Setfield.compose()
#= /home/jan/.julia/packages/Setfield/gMRkV/src/sugar.jl:113 =#
#4###_#372 = Setfield.set(identity(2), #3#lens, begin
            #= REPL[3]:1 =#
            3
        end)

end

tkf commented 5 years ago

@jw3126

  • Stuff like DiffEqBase.isinplace should be handled elsewhere.

Totally agree. I mentioned DiffEqBase.isinplace just to emphasise that I may not be using many "demo" lenses I added.

(But I just realized that @lens last(_) is useful until #15 is fully resolved.)

  • Might also make sense to actually print this type as typeof(@lens myfunction(_))?

We can. But we need to be extremely careful in the implementation: https://github.com/JuliaLang/julia/issues/29428

  • This is PR will break some code

Ah, good point. I also should test that code like @set x[length(x) - 1] = 0 still works. Related to this, maybe we should ban calls with arity > 1 outside []?

I wonder if there is a mechanism to hook into e.g. @set, @lens, ... macro and so rules like this could live in an external package. E.g. Kaleido.

I guess we need to build some kind of hook system to do that especially for parsing part of @lens and @set. But I think that'll be way more complicated than just adding @lens f(_)-support directly. This is one of the reason why I opened a PR rather than experimenting in Kaleido.

@cstjean Interesting observation! So Setfield will be yet another proof that Julia is the LISP for the 21st century :)

tkf commented 5 years ago

There are probably tons of corner cases like

julia> first(42)
42

Ah... Of course. Or maybe there should be Base.setindex(::Number, v, i) in Julia Base :laughing: . It would be an "interesting" exercise to try PRing it.

jw3126 commented 5 years ago
  • Might also make sense to actually print this type as typeof(@lens myfunction(_))?

We can. But we need to be extremely careful in the implementation: JuliaLang/julia#29428

Oh wow I somehow never ran into that bug. Yeah I think it's not worth to touch the printing of that type. Can you document how to do the set overloading?

jw3126 commented 5 years ago

This looks good to me! Shall we merge it?

tkf commented 5 years ago

Yes, I implemented what I wanted to add. Please go ahead and merge. Thanks for reviewing this!

jw3126 commented 5 years ago

Thanks for the PR. I like your idea a lot!

jw3126 commented 5 years ago

Feel free to tag a release if you need one!