search

jw3126 / Setfield.jl

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

[RFC] monkey patch setindex #107

Closed jw3126 closed 4 years ago

jw3126 commented 4 years ago

It is a recurring annoyance for me that I want to @set arr::Array and cannot do it.

Should we allow doing that? Should we use our own variant of Base.setindex? Since BangBang also needs this and implements it much better then this PR, should that function be part of ConstructionBase? Or should it be in a NoBang package?

What do you think @tkf?

tkf commented 4 years ago

I made #108 to add BangBang-like type-widening semantics to Setfield.setindex. I think it'd be more consistent with how other lenses work (e.g., set((1, 2), (@lens _[1]), 1.23)). What do you think?

jw3126 commented 4 years ago

I think it would be a good idea to move this to a separate package in the JuliaObjects organization. I am fine with ImmutableInterfaces.jl.

tkf commented 4 years ago

After implementing #108, I realized that it's hard to do this outside of Base. We either need to:

  1. Cover whole Julia ecosystem via @require as I did for StaticArrays in #108, or
  2. Convince other packages to define both ImmutableInterfaces.setindex and Base.setindex.

But since option 1 is the only direction that is doable on our side, I guess we don't have much choice but to use option 1...

ATM, there is a small "circular" dependency from BangBang to NoBang for implementing append(xs::AbstractVector, ys::AbstractVector): https://github.com/tkf/BangBang.jl/blob/6296b2a87d7051830a1a836b13ba3dd4d74ee052/src/NoBang/base.jl#L48. I need to think about doing this efficiently for completely decoupling NoBang from BangBang.

Meanwhile, how about just merging this PR as-is? I guess there are not many demerits for doing this?

jw3126 commented 4 years ago

I agree. Can you maybe rebase JuliaLang/julia#33495 and we try to convince people to merge it?

tkf commented 4 years ago

I resolved the conflict and also ping people in #arrays channel of slack. (But this never worked...)