diff --git a/CHANGELOG.md b/CHANGELOG.md index 3366fd904c..b59f8b41bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ This project adheres to [Semantic Versioning](https://semver.org/). ### Added - [#1508](https://github.com/plotly/dash/pull/1508) Fix [#1403](https://github.com/plotly/dash/issues/1403): Adds an x button to close the error messages box. +- [#1525](https://github.com/plotly/dash/pull/1525) Adds support for callbacks which have overlapping inputs and outputs. Combined with `dash.callback_context` this addresses many use cases which require circular callbacks. ### Changed - [#1503](https://github.com/plotly/dash/pull/1506) Fix [#1466](https://github.com/plotly/dash/issues/1466): loosen `dash[testing]` requirements for easier integration in external projects. This PR also bumps many `dash[dev]` requirements. diff --git a/dash-renderer/src/actions/dependencies.js b/dash-renderer/src/actions/dependencies.js index b83015dbf8..f88f36b003 100644 --- a/dash-renderer/src/actions/dependencies.js +++ b/dash-renderer/src/actions/dependencies.js @@ -239,7 +239,6 @@ function validateDependencies(parsedDependencies, dispatchError) { }); findDuplicateOutputs(outputs, head, dispatchError, outStrs, outObjs); - findInOutOverlap(outputs, inputs, head, dispatchError); findMismatchedWildcards(outputs, inputs, state, head, dispatchError); }); } @@ -364,31 +363,21 @@ function findDuplicateOutputs(outputs, head, dispatchError, outStrs, outObjs) { }); } -function findInOutOverlap(outputs, inputs, head, dispatchError) { - outputs.forEach((out, outi) => { - const {id: outId, property: outProp} = out; - inputs.forEach((in_, ini) => { - const {id: inId, property: inProp} = in_; - if (outProp !== inProp || typeof outId !== typeof inId) { - return; - } - if (typeof outId === 'string') { - if (outId === inId) { - dispatchError('Same `Input` and `Output`', [ - head, - `Input ${ini} (${combineIdAndProp(in_)})`, - `matches Output ${outi} (${combineIdAndProp(out)})` - ]); - } - } else if (wildcardOverlap(in_, [out])) { - dispatchError('Same `Input` and `Output`', [ - head, - `Input ${ini} (${combineIdAndProp(in_)})`, - 'can match the same component(s) as', - `Output ${outi} (${combineIdAndProp(out)})` - ]); +function checkInOutOverlap(out, inputs) { + const {id: outId, property: outProp} = out; + return inputs.some(in_ => { + const {id: inId, property: inProp} = in_; + if (outProp !== inProp || typeof outId !== typeof inId) { + return false; + } + if (typeof outId === 'string') { + if (outId === inId) { + return true; } - }); + } else if (wildcardOverlap(in_, [out])) { + return true; + } + return false; }); } @@ -749,15 +738,52 @@ export function computeGraphs(dependencies, dispatchError) { return idList; } + /* multiGraph is used only for testing circularity + * + * Each component+property that is used as an input or output is added as a node + * to a directed graph with a dependency from each input to each output. The + * function triggerDefaultState in index.js then checks this graph for circularity. + * + * In order to allow the same component+property to be both an input and output + * of the same callback, a two pass approach is used. + * + * In the first pass, the graph is built up normally with the exception that + * in cases where an output is also an input to the same callback a special + * "output" node is added and the dependencies target this output node instead. + * For example, if `slider.value` is both an input and an output, then the a new + * node `slider.value__output` will be added with a dependency from `slider.value` + * to `slider.value__output`. Splitting the input and output into separate nodes + * removes the circularity. + * + * In order to still detect other forms of circularity, it is necessary to do a + * second pass and add the new output nodes as a dependency in any *other* callbacks + * where the original node was an input. Continuing the example, any other callback + * that had `slider.value` as an input dependency also needs to have + * `slider.value__output` as a dependency. To make this efficient, all the inputs + * and outputs for each callback are stored during the first pass. + */ + + const outputTag = '__output'; + const duplicateOutputs = []; + const cbIn = []; + const cbOut = []; + + function addInputToMulti(inIdProp, outIdProp, firstPass = true) { + multiGraph.addNode(inIdProp); + multiGraph.addDependency(inIdProp, outIdProp); + // only store callback inputs and outputs during the first pass + if (firstPass) { + cbIn[cbIn.length - 1].push(inIdProp); + cbOut[cbOut.length - 1].push(outIdProp); + } + } + parsedDependencies.forEach(function registerDependency(dependency) { const {outputs, inputs} = dependency; - // multiGraph - just for testing circularity - - function addInputToMulti(inIdProp, outIdProp) { - multiGraph.addNode(inIdProp); - multiGraph.addDependency(inIdProp, outIdProp); - } + // new callback, add an empty array for its inputs and outputs + cbIn.push([]); + cbOut.push([]); function addOutputToMulti(outIdFinal, outIdProp) { multiGraph.addNode(outIdProp); @@ -791,15 +817,29 @@ export function computeGraphs(dependencies, dispatchError) { outputs.forEach(outIdProp => { const {id: outId, property} = outIdProp; + // check if this output is also an input to the same callback + const alsoInput = checkInOutOverlap(outIdProp, inputs); if (typeof outId === 'object') { const outIdList = makeAllIds(outId, {}); outIdList.forEach(id => { - addOutputToMulti(id, combineIdAndProp({id, property})); + const tempOutIdProp = {id, property}; + let outIdName = combineIdAndProp(tempOutIdProp); + // if this output is also an input, add `outputTag` to the name + if (alsoInput) { + duplicateOutputs.push(tempOutIdProp); + outIdName += outputTag; + } + addOutputToMulti(id, outIdName); }); - addPattern(outputPatterns, outId, property, finalDependency); } else { - addOutputToMulti({}, combineIdAndProp(outIdProp)); + let outIdName = combineIdAndProp(outIdProp); + // if this output is also an input, add `outputTag` to the name + if (alsoInput) { + duplicateOutputs.push(outIdProp); + outIdName += outputTag; + } + addOutputToMulti({}, outIdName); addMap(outputMap, outId, property, finalDependency); } }); @@ -814,6 +854,25 @@ export function computeGraphs(dependencies, dispatchError) { }); }); + // second pass for adding new output nodes as dependencies where needed + duplicateOutputs.forEach(dupeOutIdProp => { + const originalName = combineIdAndProp(dupeOutIdProp); + const newName = originalName.concat(outputTag); + for (var cnt = 0; cnt < cbIn.length; cnt++) { + // check if input to the callback + if (cbIn[cnt].some(inName => inName === originalName)) { + /* make sure it's not also an output of the callback + * (this will be the original callback) + */ + if (!cbOut[cnt].some(outName => outName === newName)) { + cbOut[cnt].forEach(outName => { + addInputToMulti(newName, outName, false); + }); + } + } + } + }); + return finalGraphs; } diff --git a/dash-renderer/src/actions/dependencies_ts.ts b/dash-renderer/src/actions/dependencies_ts.ts index a053d4ffd8..0314f4f1ad 100644 --- a/dash-renderer/src/actions/dependencies_ts.ts +++ b/dash-renderer/src/actions/dependencies_ts.ts @@ -167,11 +167,15 @@ export const getReadyCallbacks = ( forEach(output => (outputsMap[output] = true), outputs); // Find `requested` callbacks that do not depend on a outstanding output (as either input or state) + // Outputs which overlap an input do not count as an outstanding output return filter( cb => - all( + all( cbp => !outputsMap[combineIdAndProp(cbp)], - flatten(cb.getInputs(paths)) + difference( + flatten(cb.getInputs(paths)), + flatten(cb.getOutputs(paths)) + ) ), candidates ); diff --git a/tests/integration/callbacks/test_basic_callback.py b/tests/integration/callbacks/test_basic_callback.py index 280604ed11..8deb0ea6b7 100644 --- a/tests/integration/callbacks/test_basic_callback.py +++ b/tests/integration/callbacks/test_basic_callback.py @@ -651,3 +651,48 @@ def update_output(n1, t1, n2, t2): assert timestamp_2.value > timestamp_1.value assert call_count.value == 4 dash_duo.percy_snapshot("button-2 click again") + + +def test_cbsc015_input_output_callback(dash_duo): + lock = Lock() + + app = dash.Dash(__name__) + app.layout = html.Div( + [html.Div("0", id="input-text"), dcc.Input(id="input", type="number", value=0)] + ) + + @app.callback( + Output("input", "value"), Input("input", "value"), + ) + def circular_output(v): + ctx = dash.callback_context + if not ctx.triggered: + value = v + else: + value = v + 1 + return value + + call_count = Value("i", 0) + + @app.callback( + Output("input-text", "children"), Input("input", "value"), + ) + def follower_output(v): + with lock: + call_count.value = call_count.value + 1 + return str(v) + + dash_duo.start_server(app) + + input_ = dash_duo.find_element("#input") + for key in "2": + with lock: + input_.send_keys(key) + + wait.until(lambda: dash_duo.find_element("#input-text").text == "3", 2) + + assert call_count.value == 2, "initial + changed once" + + assert not dash_duo.redux_state_is_loading + + assert dash_duo.get_logs() == [] diff --git a/tests/integration/clientside/assets/clientside.js b/tests/integration/clientside/assets/clientside.js index eaee0e6a09..71e8dc0abe 100644 --- a/tests/integration/clientside/assets/clientside.js +++ b/tests/integration/clientside/assets/clientside.js @@ -81,5 +81,22 @@ window.dash_clientside.clientside = { states_list_to_str: function(val0, val1, st0, st1) { return JSON.stringify(dash_clientside.callback_context.states_list); + }, + + input_output_callback: function(inputValue) { + const triggered = dash_clientside.callback_context.triggered; + if (triggered.length==0){ + return inputValue; + } else { + return inputValue + 1; + } + }, + + input_output_follower: function(inputValue) { + if (!window.callCount) { + window.callCount = 0 + } + window.callCount += 1; + return inputValue.toString(); } }; diff --git a/tests/integration/clientside/test_clientside.py b/tests/integration/clientside/test_clientside.py index f3239ba6bc..c8b2629cad 100644 --- a/tests/integration/clientside/test_clientside.py +++ b/tests/integration/clientside/test_clientside.py @@ -683,3 +683,37 @@ def test_clsd013_clientside_callback_context_states_list(dash_duo): '{"id":{"in1":2},"property":"value","value":"test 2"}]]' ), ) + + +def test_clsd014_input_output_callback(dash_duo): + app = Dash(__name__, assets_folder="assets") + + app.layout = html.Div( + [html.Div(id="input-text"), dcc.Input(id="input", type="number", value=0)] + ) + + app.clientside_callback( + ClientsideFunction( + namespace="clientside", function_name="input_output_callback" + ), + Output("input", "value"), + Input("input", "value"), + ) + + app.clientside_callback( + ClientsideFunction( + namespace="clientside", function_name="input_output_follower" + ), + Output("input-text", "children"), + Input("input", "value"), + ) + + dash_duo.start_server(app) + + dash_duo.find_element("#input").send_keys("2") + dash_duo.wait_for_text_to_equal("#input-text", "3") + call_count = dash_duo.driver.execute_script("return window.callCount;") + + assert call_count == 2, "initial + changed once" + + assert dash_duo.get_logs() == [] diff --git a/tests/integration/devtools/test_callback_validation.py b/tests/integration/devtools/test_callback_validation.py index 4f57e01254..3be7604b84 100644 --- a/tests/integration/devtools/test_callback_validation.py +++ b/tests/integration/devtools/test_callback_validation.py @@ -5,6 +5,7 @@ import dash_html_components as html from dash import Dash from dash.dependencies import Input, Output, State, MATCH, ALL, ALLSMALLER +from dash.testing import wait debugging = dict( debug=True, use_reloader=False, use_debugger=True, dev_tools_hot_reload=False @@ -91,9 +92,7 @@ def x(a): dash_duo.start_server(app, **debugging) - # the first one is just an artifact... the other 4 we care about specs = [ - ["Same `Input` and `Output`", []], [ "Callback item missing ID", ['Input[0].id = ""', "Every item linked to a callback needs an ID"], @@ -252,33 +251,9 @@ def y2(c): dash_duo.start_server(app, **debugging) - specs = [ - [ - "Same `Input` and `Output`", - [ - 'Input 0 ({"b":MATCH,"c":1}.children)', - "can match the same component(s) as", - 'Output 1 ({"b":MATCH,"c":1}.children)', - ], - ], - [ - "Same `Input` and `Output`", - [ - 'Input 0 ({"a":1}.children)', - "can match the same component(s) as", - 'Output 0 ({"a":ALL}.children)', - ], - ], - [ - "Same `Input` and `Output`", - ["Input 0 (c.children)", "matches Output 1 (c.children)"], - ], - [ - "Same `Input` and `Output`", - ["Input 0 (a.children)", "matches Output 0 (a.children)"], - ], - ] - check_errors(dash_duo, specs) + # input/output overlap is now legal, shouldn't throw any errors + wait.until(lambda: ~dash_duo.redux_state_is_loading, 2) + assert dash_duo.get_logs() == [] def test_dvcv006_inconsistent_wildcards(dash_duo): @@ -814,3 +789,36 @@ def test_dvcv015_multipage_validation_layout(validation, dash_duo): dash_duo.wait_for_text_to_equal("#page-2-display-value", 'You have selected "LA"') assert not dash_duo.get_logs() + + +def test_dvcv016_circular_with_input_output(dash_duo): + app = Dash(__name__) + + app.layout = html.Div( + [html.Div([], id="a"), html.Div(["Bye"], id="b"), html.Div(["Hello"], id="c")] + ) + + @app.callback( + [Output("a", "children"), Output("b", "children")], + [Input("a", "children"), Input("b", "children"), Input("c", "children")], + ) + def c1(a, b, c): + return a, b + + @app.callback(Output("c", "children"), [Input("a", "children")]) + def c2(children): + return children + + dash_duo.start_server(app, **debugging) + + specs = [ + [ + "Circular Dependencies", + [ + "Dependency Cycle Found:", + "a.children__output -> c.children", + "c.children -> a.children__output", + ], + ] + ] + check_errors(dash_duo, specs)