jw3126 / Setfield.jl

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

Change call signature to get(obj, lens), set(obj, lens, val) and modify(f, obj, lens)? #42

Closed tkf closed 6 years ago

tkf commented 6 years ago

Re-reading the docstrings, I remember one gotcha in using Setfield: the order of arguments seems to be inconsistent with the choice in Base.

For example, you have get(collection, key, default) for associative collections but it's get(l::Lens, obj) in Setfield. Considering that lens is similar to key for the collections, it feels like get(obj, l::Lens) would be more natural to Julia users.

Similarly, you have setproperty!(value, name::Symbol, x) and setfield!(value, name::Symbol, x) but it's set(l::Lens, obj, val) in Setfield. Why don't we use set(obj, l::Lens, val)?

Also, get(f::Function, collection, key) is kind of similar to modify (at least argument types are) so modify(f, obj, lens) seems to be the natural choice.

Keeping Setfield natural to Julia users seems to be more important than (say) compatibility with lens in Haskell. But maybe there are some important reason they chose that particular order and you want to keep it? (I only know very little about Haskell.) It would be fun to find out if so.

jw3126 commented 6 years ago

Historically I only planned to have the @set macro and realized only during implementation that this could be done using lenses. So lenses started their live as an implementation detail and I did not bother thinking much about argument order. Mimicking Haskell is also not a goal. For instance set, get, modify are called set, view, over in Haskell.

So there is no good reason for the current order and I agree with you, that your order is more julian. If you want to change the order, that's fine with me.

jw3126 commented 6 years ago

By the way, we should also think about what is a reasonable composition order. I think the following is a good order:

la = @lens _.a
lb = @lens _.b
lab = @lens _.a.b
@assert lab == la ∘ lb
tkf commented 6 years ago

What do you mean by composition order? Are you talking about (_.a.b).c vs _.a.(b.c)? But I thought you already benchmarked it?:

https://github.com/jw3126/Setfield.jl/blob/6b6eee812a110ac91dfa66e0ee4c27f13d131152/src/lens.jl#L142-L146

Also, I'm not sure if we want to overload function composition operator . Isn't it kind of an "implementation detail" in Haskell? My understanding of Haskell's lens is mostly based on Simon Peyton Jones's talk https://skillsmatter.com/skillscasts/4251-lenses-compositional-data-access-and-manipulation which explains how the "simplified" lens

type Lens' s a = forall f. Functor f
                        => (a -> f a) -> s -> f s

works. It makes sense to use function composition for composing this lens because it is function composition. But we are implementing lens with set and get so I'm wondering if it makes sense to use . Maybe it does (as it's isomorphic to Lens')? I just don't know.

Just thinking out loud, another way to implement a "sugar" for composing lenses would be something like

la = @lens _.a
lb = @lens _.b
lab = @lens _.$la.$lb

which is rather uglier and longer than la ∘ lb in this case but handy if you want to write @lens _.$la.c.d[3].$lb. Also, it reduces API that users have to remember.

jw3126 commented 6 years ago

I was talking about the argument order of compose(l1,l2) and also about sugar ∘(l1,l2) = compose(l1,l2). I am not a Haskell expert, but I would be surprised, if Haskell world considers this an implementation detail. Composition of lenses is a very natural operation. Anyway fixing composition order should be a seperate PR, would just be nice to do both breaking changes in the same release.

tkf commented 6 years ago

Oh, I totally agree that the composition of lenses are natural (I use it and I really like it). What I was not sure about was overloading function composition or not. I mean, why not * or or something else?

I was talking about the argument order of compose(l1,l2)

Ah, I see. So now it's

julia> la = @lens _.a
       lb = @lens _.b
       Setfield.compose(la, lb)
(@lens _.b.a)

but you are proposing to change it to

julia> Setfield.compose(la, lb)
(@lens _.a.b)

Yeah, that makes sense.

jw3126 commented 6 years ago

Ah okay now I understand. I think the reason for . in Haskell is that lenses form a category, where

So . in should not be thought of as just function composition, but more generally as composition in some category. And is the traditional mathematical notation for composition in a category. So this would be an argument for . However Julia is not so close to category theory, so if there are argumentes for other notation, I am open to use something else.

tkf commented 6 years ago

Got it. Now suddenly seems to be the only obvious choice :)

Not sure if we need to pay attention to this, but did you notice that * (and +) are parsed differently than other binary operators?:

julia> dump(:(a * b * c))
Expr
  head: Symbol call
  args: Array{Any}((4,))
    1: Symbol *
    2: Symbol a
    3: Symbol b
    4: Symbol c

julia> dump(:(a ∘ b ∘ c))
Expr
  head: Symbol call
  args: Array{Any}((3,))
    1: Symbol ∘
    2: Expr
      head: Symbol call
      args: Array{Any}((3,))
        1: Symbol ∘
        2: Symbol a
        3: Symbol b
    3: Symbol c

But it's the order implemented by current compose anyway so I suppose it's not a big deal?

jw3126 commented 6 years ago

I was not aware of this, but actually it is the parsing of +, * and not that surprises me here.

tkf commented 6 years ago

Yeah, same here. When I found it, I wondered if there were some optimization they could do with this AST.