Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Input output callback #1525

Merged
merged 16 commits into from
Jan 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
127 changes: 93 additions & 34 deletions dash-renderer/src/actions/dependencies.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
}
Expand Down Expand Up @@ -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;
});
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
});
Expand All @@ -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;
}

Expand Down
8 changes: 6 additions & 2 deletions dash-renderer/src/actions/dependencies_ts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
Expand Down
45 changes: 45 additions & 0 deletions tests/integration/callbacks/test_basic_callback.py
Original file line number Diff line number Diff line change
Expand Up @@ -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() == []
17 changes: 17 additions & 0 deletions tests/integration/clientside/assets/clientside.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
};
34 changes: 34 additions & 0 deletions tests/integration/clientside/test_clientside.py
Original file line number Diff line number Diff line change
Expand Up @@ -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() == []
66 changes: 37 additions & 29 deletions tests/integration/devtools/test_callback_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"],
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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)