callback! function refactoring

[waralex]

Refactoring of callbacks

• string macro callid removed
• separate classes Input, Output and State added
• signature of callback! changed to

function callback!(func::Union{Function, ClientsideFunction, String},
     app::DashApp,
     output::Union{Vector{Output}, Output},
     input::Union{Vector{Input}, Input},
     state::Union{Vector{State}, State} = State[]
     )

• the order of arguments in the callback handler is changed to inputs..., states...
• CallbackId renamed to CallbackDeps and removed from exports (i.e. from public API)

Now the only way to set the callback is like:

callback!(app, 
        Output("display-all-of-the-values", "value"),
        [Input("x","value"), Input("y","value"), Input("x-plus-y","value"), Input("x-plus-y-div-2","value")],
        [State("x","value"), State("y","value")]
        ) do args...
    return join(string.(args), "\n")
end

The ability to pass a dictionary(named tuple in the case of Julia) as id will be done in a separate PR associated with pattern-matching. In this PR, I do not add new functionality, only bring the existing one to its normal appearance

[alexcjohnson]

Very nice - what you've implemented here is very similar to the current Python version, which will really help.

There's also a PR open right now on the Python side to allow all the outputs, inputs, and states to be passed without nesting them in a list, with the constraint that outputs come first, then inputs, then states: plotly/dash#1180 (it's named "Single Input" as it started out just allowing a single input to be unnested, but was then generalized, and we do intend to accept it once it's finished).

I suppose we could always add this as an additional method (which is effectively what we're doing in Python with that PR) but I wonder if - since we're free of the backward-compatibility constraint here - we shouldn't just make the unnested form the only way to do it?

Unfortunately I guess that would require a manual type check to find the first Input and State and throw if they're out of order - I was hoping Julia would allow defining a function with a signature with multiple varargs like

f(app::dashApp, outputs::Output..., inputs::Input..., states::State...)

but apparently not 🤷

The only behavior I can think of that this would prevent is returning a single output as a one-item list. That's a pretty weird use case, but theoretically I can imagine wanting it for creating callbacks programmatically if sometimes there's one output and sometimes more, but your function always returns a list.

So I guess unless you can think of another simple way to disambiguate a single output in a list or not, we should keep the implementation you have here and add the varargs form as an alternative, just like we're doing in Python. Which means it need not happen in this PR, but it could if you like.

[waralex]

@alexcjohnson

I was hoping Julia would allow defining a function with a signature with multiple varargs like

Unfortunately the function can only have one varargs argument

The only behavior I can think of that this would prevent is returning a single output as a one-item list.

To be honest, I didn't really understand the problem, could you explain it in more detail and give an example?

In principle, there is nothing complicated about making a function accept an arbitrary number of both individual elements and arrays of elements.

callback!(app,
    Output("f","f"),
   [Output("ff","fff"), ...],
   Output("gg","gg"),
   Input("gg","ggg")
  .....
) ....

[waralex]

Unfortunately I guess that would require a manual type check to find the first Input and State and throw if they're out of order

With this approach, it would make sense to ensure instead that arguments are passed to the callback in the same order as they are defined in the function. But this is different from the behavior in Python, so it's probably just a dream. But it would allow such things to be possible:

callback!(app, 
    Output("my-div", "children"),
    State("my-input", "value"), #inportant value
    make_inputs()... #long generated list of inputs
) do important_state, args...
    return args[important_state]
end

[alexcjohnson]

If the only syntax we provide is varargs, then a single-output callback would look like:

callback!(app,
    Output("out1", "children"),
    Input("in1", "value")
) do input
    return val
end

And a multi-output callback would look like:

callback!(app,
    Output("out1", "children"),
    Output("out2", "children"),
    Input("in1", "value")
) do input
    return [val1, val2]
end

But what if you want your single output to be nested in a list/vector? With the way this PR looks today, it would be:

callback!(app,
    [Output("out1", "children")],
    Input("in1", "value")
) do input
    return [val]
end

but there would be no way to specify this in the varargs form. Normally you don't want this, but maybe you would if you're programmatically generating the callback function and you don't know ahead of time how many outputs it will have. It's a weird case (and there's probably even less use for it after we have pattern-matching callbacks) but the only way I see to allow it is if Output(...) and [Output(...)] are treated differently.

[alexcjohnson]

With this approach, it would make sense to ensure instead that arguments are passed to the callback in the same order as they are defined in the function. But this is different from the behavior in Python, so it's probably just a dream.

Yes, in principle that could be supported, but as I said in plotly/dash#1180 (comment):

I think it's still important to have the items in order: outputs, inputs, state, rather than mixing them up. It would be super confusing to have outputs mixed in with the others, so definitely we need those to be first. Less confusing to have inputs and state mixed up, and I can see the rationale of grouping related items, but the distinction between inputs and state is important enough to the logic of the callback that I think it's worth keeping them separate.

Let's enforce the ordering outputs..., inputs..., states... for now, we can always loosen the constraint later but it would be a breaking change to take a loose constraint and make it stricter.

[waralex]

Normally you don't want this, but maybe you would if you're programmatically generating the callback function and you don't know ahead of time how many outputs it will have

I understood. Thank you, this is an interesting case, I didn't think about it. In fact, Dash now stores output always as an array. and in what form it is given to the frontend is determined based on array size:

function output_string(deps::CallbackDeps)
    if length(deps.output) == 1
        return dependency_string(deps.output[1])
    end
    return ".." *
    join(dependency_string.(deps.output), "...") *
    ".."
end

So I should learn more about how this works if an array of a single element is returned from the callback. Apparently the problem is deeper and I need to fix this part too.
What do you think about allowing to pass both single elements and arrays to varargs version?

[waralex]

I tend to think that it is most convenient and understandable to still have 2 versions. One as now and the second with varagrs, in which only individual elements can be passed to varargs. The ability to mix single elements and arrays in varargs will confuse the user.

I.e. 2 overloads:

function callback!(func::Union{Function, ClientsideFunction, String},
     app::DashApp,
     output::Union{Vector{Output}, Output},
     input::Union{Vector{Input}, Input},
     state::Union{Vector{State}, State} = State[]
     )
end
function callback!(func::Union{Function, ClientsideFunction, String},
     app::DashApp,
     deps::Dependency...
     )
     
end

[waralex]

@alexcjohnson in 7c36096 I added a flat version of callback! and fixed the work with a single element array output

[alexcjohnson]

This looks great! Very nicely done, and excellent tests. 💃