support two-argument functions in optics

[aplavin]

For now, contains implementation and a single specific delete method as an example.
If others agree this extension is a good idea, will add more methods and tests.

[jw3126]

Thanks for @aplavin for exploring this direction, looks interesting!

If others agree this extension is a good idea, will add more methods and tests.

So the first example does not look very useful to me. But there might be other more practical cases. Do you have specific examples in mind, that might be practically useful?

[aplavin]

A few usecases I have in mind:

Suppose there is a function that modifies the value located by an optic:

function f(xs, o)
    vals = o.(xs)
    ... compute new_vals ...
    set.(xs, o, new_vals)
end

Then, one can call it like f([(a=1,), (a=2,), ...], @optic _.a). Want to operate on logarithms - also fine: f([(a=1,), (a=2,), ...], @optic log(_.a)).
But what if I need the function to operate on scaled values? f([(a=1,), (a=2,), ...], @optic _.a * 1e10) doesn't work without this PR. Same with units: f([(a=1u"cm",), (a=2u"cm",), ...], @optic ustrip(u"m", _.a)).

first(_, n) is also useful. Suppose the function f above works with 2d coordinates. Then f([(coords=(1, 1),), (coords=(2, 1),), ...], @optic _.coords) is fine. But how to apply it when data contain 3d coordinates? @optic first(_.coords, 2) solves this.

Some of these examples require defining corresponding set methods. I don't propose to define many function-lenses in this PR, and some (like ustrip) are impossible without depending on other packages. They are easy to define by downstream users, unlike the groundwork on optic parsing I propose here.

[jw3126]

Thanks, @aplavin that makes sense to me. In code like @set a*b = c. What is returned a_new or b_new?
Also the first example can currently be expressed as @optics _.coords[1:2].

[aplavin]

In code like @set a*b = c. What is returned a_new or b_new?

This should work or not work exactly as before - seems like c is returned?... My PR only addresses unambigous cases: the @optic macro, and only one of the function arguments can contain _.

[aplavin]

Also the first example can currently be expressed as @Optics _.coords[1:2].

Indeed, it can. I agree that first is less needed here.
Btw, I cannot make this work in current Accessors.jl:

julia> x = (1, 2, 3)
julia> @set x[1:2] = (10, 20)
ERROR: MethodError: no method matching setindex(::Tuple{Int64, Int64, Int64}, ::Tuple{Int64, Int64}, ::UnitRange{Int64})

But that becomes orthogonal to this PR.

[jw3126]

Ok, I am still not 100% convinced, but we can give it a shot. I also have quite some use cases, in the spirit of stripping units or adjusing scale. But usually, these cases are more complex and would not benefit from this PR. What I need from time to time is flatten parts of a nested struct into a rescaled vector that conforms some optimization API. In these cases and I define a custom optic directly.
So my suggestion would be:

• We can merge this PR (and follow up PRs if required)
• You report back in a few months, whether you actually find this feature useful
  Does that sound good to you?

[jw3126]

BTW I just checked:

julia> using Accessors

julia> a = 1
1

julia> b = 2
2

julia> c = 3
3

julia> @set a*b = c
3

This is somewhat surprising. This happens because @set a*b = c is parsed as obj = a*b, optic=identity:

julia> @macroexpand @set a*b = c
:(var"#2###_#275" = (Accessors.set)(a * b, (identity)((Accessors.opticcompose)()), begin
              #= REPL[6]:1 =#
              c
          end))

I consider it a bug and if your PR incidentally turns it into an error, that's fine.

[aplavin]

What I need from time to time is flatten parts of a nested struct into a rescaled vector that conforms some optimization API.

Oh, that's also useful! And I agree that this PR wouldn't really help here.
I sometimes do these btw:

So that calling map(optics, array_of_objects) returns a flat table with requested columns. But this is probably waaay to opinionated to be included in Accessors.jl itself.

Definitions

Sorry for formatting, code copied from a notebook.

# ╔═╡ 3de7af46-7375-44fa-857d-f730fcbae5a6
(nt::NamedTuple)(x) = map(f -> f(x), nt)

# ╔═╡ 6a115e94-9627-45ab-aed9-5950482faf45
begin
	flatten_composed(l::ComposedFunction) = (flatten_composed(l.inner)..., flatten_composed(l.outer)...)
	flatten_composed(l) = (l,)
end

# ╔═╡ 855f7f74-1328-46fa-bca6-e1667c61b32b
begin
	flat_key(l::ComposedFunction; nlast=1) = nlast == 1 ? flat_key(flatten_composed(l)[end]) :
		nlast == 2 ? Symbol(flat_key(flatten_composed(l)[end-1]), :(_), flat_key(flatten_composed(l)[end])) :
		nlast == 3 ? Symbol(flat_key(flatten_composed(l)[end-2]), :(_), flat_key(flatten_composed(l)[end-1]), :(_), flat_key(flatten_composed(l)[end])) :
		nlast == 4 ? Symbol(flat_key(flatten_composed(l)[end-3]), :(_), flat_key(flatten_composed(l)[end-2]), :(_), flat_key(flatten_composed(l)[end-1]), :(_), flat_key(flatten_composed(l)[end])) :
		nlast == 5 ? Symbol(flat_key(flatten_composed(l)[end-4]), :(_), flat_key(flatten_composed(l)[end-3]), :(_), flat_key(flatten_composed(l)[end-2]), :(_), flat_key(flatten_composed(l)[end-1]), :(_), flat_key(flatten_composed(l)[end])) :
		error("not implemented")
	flat_key(l::Accessors.PropertyLens{K}) where {K} = K
	flat_key(l::Accessors.IndexLens) = only(l.indices)
	flat_key(l::Function) = nameof(l)
end

# ╔═╡ 210aeab0-5ec9-48fc-8e97-bd325e51e853
function w_flat_keys(ls)
	res = flat_key.(ls)
	i = 1
	while !allunique(res)
		i += 1
		dct = @p group(res, ls) |>
			mapmany() do gr
				length(gr) == 1 && return []
				keys = flat_key.(gr; nlast=i)
				gr .=> keys
			end |>
			Dict
		res = get.(Ref(dct), ls, res)
	end
	return res .=> ls
end

# ╔═╡ 06eb1166-3697-49ce-b5c6-e04df608f5cb
begin
	to_flat_nt(x::NamedTuple) = x
	to_flat_nt(x::Tuple) = (;w_flat_keys(x)...)
	to_flat_nt(x::AbstractVector) = to_flat_nt(Tuple(x))
	to_flat_nt(x) = to_flat_nt((x,))
end

[aplavin]

So my suggestion would be:

We can merge this PR (and follow up PRs if required)
You report back in a few months, whether you actually find this feature useful
Does that sound good to you?

That totally works for me!

[aplavin]

In these cases and I define a custom optic directly.

Do you have any neat or simple examples? I'm not really experienced with custom optics, outside of defining set for existing functions.

[jw3126]

In these cases and I define a custom optic directly.

Do you have any neat or simple examples? I'm not really experienced with custom optics, outside of defining set for existing functions.

My code looks close to your code, with one addition. I often need constraint optimization and I often incorportate the constraints by transforming my variables. For instance, say I want to minimize f(x) where x is in the interval [lo,hi], but my solver only supports unconstraint optimization. Then one can define an optic like:

struct Stretch{T}; lo::T; hi::T; end
(o::Stretch)(x_bounded) = # use bijection from [lo, hi] to IR
Accessors.set(_, optic::Stretch, y_unbound) = # use inverse bijection

function optimize(f, obj0, lens)
    v0 = lens(obj0)
    function _f(v)
         obj = set(obj0, lens, v)
         f(obj)
    end
    _sol = LibThatExpectsVectors.optimize(_f, v0)
    set(obj0, lens, _sol)
end

optimize(f, 0.5, Stretch(0,1))

Then I build more complicated optics from such building blocks, similar to your code above.

[aplavin]

First I thought that it's better to add more lenses in the same PR, but now I believe it's best to merge this as is. There is one example and a test, confirming that the new machinery works fine.

I think I'm going to propose new lenses separately, so that potential discussions of what to include don't affect the two-argument function parsing itself.
Mostly, I'm thinking about arithmetic, introduce set methods for

• single-argument functions in Base, like exp, log, sqrt, angle,
• two-argument functions (based on this PR), like +, *, ^.

For example, this lets to define a stretch (sigmoid) optic directly:

julia> x = 2
julia> o = @optic 1/(1 + exp(-_))

julia> o(x)
0.8807970779778823

julia> set(x, o, 0.999)
6.906754778648465

[aplavin]

This happens because @set ab = c is parsed as obj = ab, optic=identity:
I consider it a bug and if your PR incidentally turns it into an error, that's fine.

This PR doesn't do anything against the case you mentioned. Furthermore, the current behavior is not that unexpected sometimes, and even tested in the testsuite:

Accessors.jl/test/test_core.jl

Line 123 in fb8fb82

t = @set T(1,2).a = 2

Still not sure if I like this or not, but it's not always a bug.

[jw3126]

This PR doesn't do anything against the case you mentioned. Furthermore, the current behavior is not that unexpected sometimes, and even tested in the testsuite:

Yes it is not the job of the PR to address this, sorry if I made that impression. It was just that if this PR broke that behavior, that would be ok to me.

Still not sure if I like this or not, but it's not always a bug.

Yes this was intended in ancient times, where we had no function lenses. But now I think it is a misfeature and I am happy to sacrifice it.

Can you add a test, that simulates a user defining mybinop and overaloading the required methods on Fix1/Fix2 ?

[aplavin]

Yes this was intended in ancient times, where we had no function lenses. But now I think it is a misfeature and I am happy to sacrifice it.

While we are at it, what do you think of making @set $(v[i]).a = 5 equivalent to vi = v[i]; @set vi.a? That is, expression in $ treated as the @set target.
Then instead of @set T(1,2).a = 2 one would write @set $(T(1,2)).a = 2

Can you add a test, that simulates a user defining mybinop and overaloading the required methods on Fix1/Fix2 ?

Added!

[jw3126]

Thanks a lot LGTM. Shall I merge?

[aplavin]

Yeah! Please register as well.

[aplavin]

You report back in a few months, whether you actually find this feature useful

It's not "a few" month, but still :)
Yes, I find this feature pretty useful - most often with arithmetic lenses, but also sometimes for string stuff or map(optic, _). Great that it was merged!

But usually, these cases are more complex and would not benefit from this PR. What I need from time to time is flatten parts of a nested struct into a rescaled vector that conforms some optimization API.

Flattening turned out to be kinda orthogonal to this PR, and as you surely know it's getall/setall.

And indeed arithmetic lenses (many of which are Base.Fix1/2) are especially nice with "flattening". Changing scales, units, doing other transformation to optimization parameters - everything is so composable.
See https://gitlab.com/aplavin/AccessibleOptimization.jl for accessors-based optimization problem specification, and https://gitlab.com/aplavin/PlutoTables.jl for accessors-based data editing UIs.