Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Support for callbacks with no outputs #2682

Conversation

yashjha123
Copy link

This PR allows callbacks to have no return values. This is the most requested feature on this repo.. Personally, I can see multiple examples where this feature can be used. It can be used to, let's say, make a post request to a website we don't need the response for, for eg. tracking clicks. Other examples, for eg, opening a new website (using clientside callbacks) and many applications.

Here's a basic example of how this might work.

from dash import Dash, html, callback, Output, Input

app = Dash(__name__)

app.layout = html.Div([
    html.H1(children='Title of Dash App', style={'textAlign':'center'}),
    html.Div(id="result"),
    html.Button(id='output-button',children="This button gives an output"),
    html.Button(id='no-output-button',children="no output button"),

])

@callback(
    Output("result","children"),
    Input("output-button","n_clicks")
)
def make_result(n_clicks):
    return f"Button with output looks like this {n_clicks}"

@callback(
    Input("no-output-button","n_clicks"),
    force_no_output=True
)
def make_result(n_clicks):
    print("Ignored",n_clicks)
    return None


if __name__ == '__main__':
    app.run(debug=True)

This new feature is built on the existing logic used for duplicate outputs. I've added conditions to the error statements that overwrite the behaviors specifically for no-output callbacks.
I imagine the final version will have warnings on the terminal and more rigid statements. However, this implementation is convincing to me to be in the production env. What do you folks think? What other changes should we make?

Contributor Checklist

  • I have broken down my PR scope into the following TODO tasks
    • Basic MVP Working
    • A generalize function wrappers(?) => 0 outputs automatically forces the attribute.
    • More Tests needs (also rewrite old tests)
  • I have run the tests locally and they passed. (refer to testing section in contributing)
  • I have added tests, or extended existing tests, to cover any new features or bugs fixed in this PR

@alexcjohnson
Copy link
Collaborator

Thanks @yashjha123, looks like a very promising start! Three things before we dig too deeply into this:

  • Would you mind rebasing or recreating this PR off the dev branch instead of master, and ensuring none of the build artifacts (dash/dash-renderer/build/*) are committed?
  • As mentioned in Allow for "null" / no output callbacks #1549 (comment) from @T4rk1n we should be able to do this without users needing to set a flag in the callback definition, just by looking at how many outputs the callback defines. If you want to generate that flag behind the scenes and pass it to the renderer that's fine, but we shouldn't make users do that.
  • Tests will be very important here, and we'll need them to cover quite a lot of situations: direct initial call, indirect initial call (ie the input is the output of another initial callback), direct user input, indirect user input, pattern matching with MATCH input, pattern matching with ALL input... we can start reviewing prior to tests but we'll need them before we can merge. Because these callbacks are only expected to have server-side effects, the best pattern to use is probably multiprocessing.Value as for example in cbmt001 - but this feature deserves a new test file in the same folder.

@@ -31,7 +31,7 @@ const defaults = {
{
test: /\.ts(x?)$/,
exclude: /node_modules/,
use: ['babel-loader', 'ts-loader'],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, I wasn't aware this could be omitted. Sounds to me as though it's safer to keep it though, to keep type checking. Per https://webpack.js.org/guides/typescript/#loader:

Note that if you're already using babel-loader to transpile your code, you can use @babel/preset-typescript and let Babel handle both your JavaScript and TypeScript files instead of using an additional loader. Keep in mind that, contrary to ts-loader, the underlying @babel/plugin-transform-typescript plugin does not perform any type checking.

@T4rk1n
Copy link
Contributor

T4rk1n commented May 9, 2024

Thank you for opening this PR, I adapted part of the code found here to not require setting a flag and allow for multiples no output callbacks.

Closing as implemented in #2822

@T4rk1n T4rk1n closed this May 9, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants