Update cycle broken after callback

[maxkfranz]

Description

After running a callback with Output and State on the graph JSON, the update cycle is broken.

This affects Scale AI and Hydro Quebec.

CC: @mtwichan @alexcjohnson @mj3cheun @jackparmer

Steps/Code to Reproduce

This is a minimal example adapted from another reproducible example from Matthew (@mtwichan). You can drag around the nodes in the demo to easily demonstrate the issue:

1. Press the print-JSON button.
2. Move a node around.
3. Press the JSON button again.
4. Note that the position changed, reflecting the drag.
5. Press the break button, which has input/output arguments on the JSON.
6. Move the node around again.
7. Press the JSON button. The position is not updated this time, or any time after this.

Notes:

• This is very likely to affect other JSON as well (e.g. data), though position is simple and clear to demonstrate.
• This may need to be filed in Dash rather than here, as this may be a general issue with Dash.
• You can drop this file into this repo in the root as something like usage.py to quickly verify.

import dash
import dash_cytoscape as cyto
import dash_html_components as html
from dash.dependencies import Input, Output, State
import json

from dash.exceptions import PreventUpdate

from copy import deepcopy

app = dash.Dash(__name__)
server = app.server

app.layout = html.Div([
    cyto.Cytoscape(
        id='cytoscape',
        elements=[
            {'data': {'id': 'one', 'label': 'Node 1'},
             'position': {'x': 50, 'y': 50}},
            {'data': {'id': 'two', 'label': 'Node 2'},
             'position': {'x': 200, 'y': 200}},
            {'data': {'source': 'one', 'target': 'two', 'label': '1 to 2'}}
        ],
        layout={'name': 'preset'}
    ),
    html.Button("Print elements JSONified", id="button-cytoscape"),
    html.Button("Break", id="button-break"),
    html.Div(id="html-cytoscape"),
])

@app.callback(
    Output("html-cytoscape", "children"),
    [Input("button-cytoscape", "n_clicks")],
    [State("cytoscape", "elements")],
)

def testCytoscape(n_clicks, elements):
    if n_clicks:
        return json.dumps(elements)

@app.callback(
    Output("cytoscape", "elements"),
    [Input("button-break", "n_clicks")],
    [State("cytoscape", "elements")],
)

def breakCytoscape(n_clicks, elements):
    if n_clicks:
        return deepcopy(elements) # happens with a deep copy or not 
    else:
        raise PreventUpdate

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

Expected Results

The update cycle should continue.

Actual Results

The update cycle is stopped.

Versions

@mtwichan, would you post your versions here for reference?

[alexcjohnson]

I think I see what's going on here:

• When we initially render the component, the elements array is used directly in the cytoscape instance.
• That means that changes made in-place to that array internally by Cytoscape are reflected in the renderer's layout object.
• But when we update elements, a new object is inserted into the renderer's layout but internally to the component the original object is patched to match the new object, rather than simply replaced by the new object.
• So after an update - any update where a new elements array was created by a callback - the one inside the Cytoscape instance is no longer the same one as in layout so updates will not be reflected there.

Another thing to note - which I imagine you all are well aware of but it's a significant limitation of this component today - is that elements does not function as an Input. So if I take the first callback above and change it to:

@app.callback(
    Output("html-cytoscape", "children"),
    Input("button-cytoscape", "n_clicks"),
    Input("cytoscape", "elements"),
)
def testCytoscape(n_clicks, elements):
    if n_clicks:
        return json.dumps(elements)

Ideally you'd be able to click the JSON button (to get the json to display at all) and then every time a node is moved you would see the new positions. But that doesn't happen: to get the updated positions to show up you have to click the JSON button again.

The ideal solution here is to watch events that cause changes in elements, and have the component call setProps({elements: newElements}) - that would propagate the changes back into layout, fixing the case raised in this PR and allowing elements to be used as an Input.

[maxkfranz]

Great. Here are some suggestions to help expedite things: (1) The most important events to listen to are, in descending order, ‘add’, ‘remove’, ‘data’, ‘position’, ’select’, ‘unselect’, and ‘viewport’. The full list is here, but not all of them would affect this the data: https://js.cytoscape.org/#events <https://js.cytoscape.org/#events> Some states, like selection, are handled with special cases in Dash Cytoscape already, though I agree that a general solution would be preferable. (2) For performance reasons, it may be useful to throttle — or ideally, debounce — the callback(s), especially if the entire graph JSON is sent for each update. This may depend on the use cases you envision the component supporting. E.g., debouncing doesn’t fit as well if the consumer wants intermediary dragging positions.

…

On Oct 25, 2021, at 14:51, Alex Johnson ***@***.***> wrote: I think I see what's going on here: When we initially render the component, the elements array is used directly in the cytoscape instance. That means that changes made in-place to that array internally by Cytoscape are reflected in the renderer's layout object. But when we update elements, a new object is inserted into the renderer's layout but internally to the component the original object is patched <https://github.com/plotly/react-cytoscapejs/blob/eadd9598a07978c66e9a0eb997cd8340cf76aa0a/src/component.js#L84> to match the new object, rather than simply replaced by the new object. So after an update - any update where a new elements array was created by a callback - the one inside the Cytoscape instance is no longer the same one as in layout so updates will not be reflected there. Another thing to note - which I imagine you all are well aware of but it's a significant limitation of this component today - is that elements does not function as an Input. So if I take the first callback above and change it to: @app.callback( Output("html-cytoscape", "children"), Input("button-cytoscape", "n_clicks"), Input("cytoscape", "elements"), ) def testCytoscape(n_clicks, elements): if n_clicks: return json.dumps(elements) Ideally you'd be able to click the JSON button (to get the json to display at all) and then every time a node is moved you would see the new positions. But that doesn't happen: to get the updated positions to show up you have to click the JSON button again. The ideal solution here is to watch events that cause changes in elements, and have the component call setProps({elements: newElements}) - that would propagate the changes back into layout, fixing the case raised in this PR and allowing elements to be used as an Input. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#159 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHRO42ITYN7P7Q5O4HPW4TUIWRLDANCNFSM5GVXIT7Q>.

[marc-vdm]

Hi all,

I've been looking to better understand my issue (#167), and I'm now thinking mine is a duplicate of this. Could you let me know if

1. you also think this is a duplicate (then I'll close mine)?
2. you managed to find a solution or workaround to this in the meantime?

My use-case really depends on being able to submit a new elements set during use, so not being able to do this would require me to find an alternative to this cytoscape library.