Merge pull request #1525 from chadaeschliman/input_output_callback

alexcjohnson · web-flow · commit c5ba38f0ae7b · 2021-01-16T09:49:10.000-05:00
Input output callback
diff --git 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
@@ -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
@@ -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<ILayoutCallbackProperty>(
                 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
@@ -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
@@ -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
@@ -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
@@ -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)
