ipycytoscape icon indicating copy to clipboard operation
ipycytoscape copied to clipboard

Published 20 hours ago • verified publisher icon cytoscape

Broken graph state after removing node

Open ktaletsk opened this issue 2 years ago • 6 comments

Bug report

cytoscapeobj.graph.remove_node(node) removes more nodes than just the node passed as the parameter. This also affects cytoscapeobj.graph.remove_node_by_id(node_id) which uses it.

https://github.com/cytoscape/ipycytoscape/blob/c72f39d5382711f22ffd1dfe0f5754602f63c9af/ipycytoscape/cytoscape.py#L287-L305

Issues seems to lie specificly in this call: https://github.com/cytoscape/ipycytoscape/blob/c72f39d5382711f22ffd1dfe0f5754602f63c9af/ipycytoscape/cytoscape.py#L296

Code for reproduction

  1. Open 'DAG example' notebook. Binder

  2. Execute all existing cells in order. The graph would look like this image

  3. Observe the list of existing nodes

cytoscapeobj.graph.nodes
_______________________
[Node(data={'id': 'n0'}, position={}),
 Node(data={'id': 'n1'}, position={}),
 Node(data={'id': 'n2'}, position={}),
 Node(data={'id': 'n3'}, position={}),
 Node(data={'id': 'n4'}, position={}),
 Node(data={'id': 'n5'}, position={}),
 Node(data={'id': 'n6'}, position={}),
 Node(data={'id': 'n7'}, position={}),
 Node(data={'id': 'n8'}, position={}),
 Node(data={'id': 'n9'}, position={}),
 Node(data={'id': 'n10'}, position={}),
 Node(data={'id': 'n11'}, position={}),
 Node(data={'id': 'n12'}, position={}),
 Node(data={'id': 'n13'}, position={}),
 Node(data={'id': 'n14'}, position={}),
 Node(data={'id': 'n15'}, position={}),
 Node(data={'id': 'n16'}, position={})]
  1. Remove node with id n3 (the bottom-right child node in the left-most graph)
cytoscapeobj.graph.remove_node_by_id('n3')

The graph now looks like this: image

Alternatively, you can remove the node from the list of nodes (which is what cytoscapeobj.graph.remove_node does internally) to the same effect:

cytoscapeobj.graph.nodes.remove(cytoscapeobj.graph.nodes[3])
  1. Inspect the list of nodes
cytoscapeobj.graph.nodes
_______________________
[Node(data={'id': 'n0'}, position={}),
 Node(data={'id': 'n1'}, position={}),
 Node(data={'id': 'n2'}, position={}),
 Node(data={'id': 'n4'}, position={}, removed=True),
 Node(data={'id': 'n5'}, position={}, removed=True),
 Node(data={'id': 'n6'}, position={}, removed=True),
 Node(data={'id': 'n7'}, position={}, removed=True),
 Node(data={'id': 'n8'}, position={}, removed=True),
 Node(data={'id': 'n9'}, position={}, removed=True),
 Node(data={'id': 'n10'}, position={}, removed=True),
 Node(data={'id': 'n11'}, position={}, removed=True),
 Node(data={'id': 'n12'}, position={}, removed=True),
 Node(data={'id': 'n13'}, position={}, removed=True),
 Node(data={'id': 'n14'}, position={}, removed=True),
 Node(data={'id': 'n15'}, position={}, removed=True),
 Node(data={'id': 'n16'}, position={}, removed=True)]
...
  1. JS console logs indicate an error immediately after running cytoscapeobj.graph.remove_node_by_id('n3'):
Uncaught (in promise) Error: Can not create second element with ID `n4`
Uncaught (in promise) Error: Can not create second element with ID `n4`
Uncaught (in promise) Error: Can not create edge `a1b4ae96-fe81-4363-ba59-23298b1a10e3` with nonexistant source `n4`

Is this the intended behavior?

Actual outcome

Nodes with ids "higher" than node requested to be deleted are getting removed=True on them and disappear from the graph.

Expected outcome

Maybe it would make more sense to make this optional, and only delete a single node by default?

Version Info

  • runtime environment: Binder (link above)
    • ipycytoscape version : 1.3.2
    • Python version: 3.7.12
    • JupyterLab version: 3.3.1

ktaletsk avatar Mar 23 '22 17:03 ktaletsk

BTW, same error with edges:

In the same 'DAG exampe' notebook, run

cytoscapeobj.graph.remove_edge_by_id('n1', 'n3')

image

cytoscapeobj.graph.edges
_______________________
[Edge(data={'source': 'n0', 'target': 'n1'}),
 Edge(data={'source': 'n1', 'target': 'n2'}),
 Edge(data={'source': 'n4', 'target': 'n5'}, removed=True),
 Edge(data={'source': 'n4', 'target': 'n6'}, removed=True),
 Edge(data={'source': 'n6', 'target': 'n7'}, removed=True),
 Edge(data={'source': 'n6', 'target': 'n8'}, removed=True),
 Edge(data={'source': 'n8', 'target': 'n9'}, removed=True),
 Edge(data={'source': 'n8', 'target': 'n10'}, removed=True),
 Edge(data={'source': 'n11', 'target': 'n12'}, removed=True),
 Edge(data={'source': 'n12', 'target': 'n13'}, removed=True),
 Edge(data={'source': 'n13', 'target': 'n14'}, removed=True),
 Edge(data={'source': 'n13', 'target': 'n15'}, removed=True)]

Clearly, there is an issue calling remove method of ipycytoscape.cytoscape.Graph.nodes and ipycytoscape.cytoscape.Graph.edges (both of the spectate.models.List type)

ktaletsk avatar Mar 23 '22 18:03 ktaletsk

Hey @ktaletsk, thanks for opening the issue and sorry for taking a while to reply to it. The person who contributed this code was @MridulS, maybe they're interested in picking this up? Or just chiming in. But anyway, that's a great found! Thank you very much for contributing with the issue, lately I haven't had much time to hack on ipycytoscape, but this issue is definitely up in my list. I can try to help you if you're interested in tackling it. I saw you're in LA? I'm here for the week! If you want we could even meet in real life. Cheers.

marimeireles avatar Mar 28 '22 22:03 marimeireles

Yeah this doesn't look right, thanks for the report.

I don't have much bandwidth for this right now but I can get back to this in a couple of weeks :) Feel free to submit a PR to fix this if you get the time before that.

MridulS avatar Mar 29 '22 09:03 MridulS

@marimeireles @MridulS here are some findings upon debugging the issue

  1. It only manifests itself if widget is rendered. Consider these 3 examples based on the code from the test suite:
    • Initial setup 
      from ipycytoscape.cytoscape import Edge, Graph, Node, CytoscapeWidget
data = {
    "nodes": [
        {"data": {"id": "0"}},
        {"data": {"id": "1"}},
        {"data": {"id": "2"}},
    ],
    "edges": [
        {"data": {"source": "0", "target": "1", "weight": "1"}},
        {"data": {"source": "0", "target": "1", "weight": "2"}},
        {"data": {"source": "1", "target": "0"}},
        {"data": {"source": "1", "target": "2"}},
        {"data": {"source": "2", "target": "0"}},
    ],
}
    • Create graph, removed node. Works as expected 
      graph = Graph()
graph.add_graph_from_json(data)
graph.remove_node_by_id("0")
graph.nodes
>> [Node(data={'id': '1'}, position={}), Node(data={'id': '2'}, position={})]
    • Same thing, but inside the CytoscapeWidget object. Still works 
      widget = CytoscapeWidget()
widget.graph = Graph()
widget.graph.add_graph_from_json(data)
widget.graph.remove_node_by_id("0")
widget.graph.nodes
>> [Node(data={'id': '1'}, position={}), Node(data={'id': '2'}, position={})]
    • Exactly the same thing, but render the widget before deleting the node. Then we observer the bug behavior 
      widget = CytoscapeWidget()
widget.graph = Graph()
widget.graph.add_graph_from_json(data)

# Render widget
widget

widget.graph.remove_node_by_id("0")
widget.graph.nodes
>>[Node(data={'id': '1'}, position={}, removed=True), Node(data={'id': '2'}, position={}, removed=True)]

With that, I think we should search for an error somewhere in the Jupyter widget.

  1. It is not completely clear to me what's happening in CytoscapeView.removeNodeView and CytoscapeView.removeEdgeView 
removeNodeView(nodeView: any) {
    nodeView.model.set('removed', true);
    nodeView.remove();
  }

I tried commenting out setting the model - the first line in the function - and the behavior was better, yet still incorrect. I was able to remove only the requested node from the list, and the state of the graph was as expected - no nodes with incorrectly set "removed: true". However, the widget was not updated automatically and I had to manually rerun the cell with the widget to get the new state.

I would like to continue debugging, but I have 2 issues:

  1. What is the type of the nodeView input argument in CytoscapeView.removeNodeView? It is set to any which makes it impossible to inspect the methods of this object.
  2. Who and where is calling CytoscapeView.removeNodeView?

ktaletsk avatar Apr 01 '22 01:04 ktaletsk

@marimeireles @MridulS I would like to fix this issue, but I'm stuck. Could you give some feedback on my comment above?

ktaletsk avatar May 06 '22 17:05 ktaletsk

I have run into this issue as well trying to update ipycytoscape widgets rendered in Jupyter. Removing any single node removes all the nodes rendered in the graph and sets each node to removed=True. Setting the nodes back manually to removed=False does not re-add them. Is there intention to address this bug?

vlvalenti avatar Dec 12 '22 17:12 vlvalenti