Open ktaletsk opened 2 years ago
BTW, same error with edges:
In the same 'DAG exampe' notebook, run
cytoscapeobj.graph.remove_edge_by_id('n1', 'n3')
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)
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.
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.
@marimeireles @MridulS here are some findings upon debugging the issue
It only manifests itself if widget is rendered. Consider these 3 examples based on the code from the test suite:
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"}},
],
}
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={})]
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.
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:
nodeView
input argument in CytoscapeView.removeNodeView
? It is set to any
which makes it impossible to inspect the methods of this object.CytoscapeView.removeNodeView
?@marimeireles @MridulS I would like to fix this issue, but I'm stuck. Could you give some feedback on my comment above?
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?
Bug report
cytoscapeobj.graph.remove_node(node)
removes more nodes than just the node passed as the parameter. This also affectscytoscapeobj.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
Open 'DAG example' notebook.![Binder](https://mybinder.org/badge_logo.svg)
Execute all existing cells in order. The graph would look like this![image](https://user-images.githubusercontent.com/8834829/159758380-d64dc2a8-a45c-443a-beaa-1aeb111af256.png)
Observe the list of existing nodes
Remove node with id
n3
(the bottom-right child node in the left-most graph)The graph now looks like this:![image](https://user-images.githubusercontent.com/8834829/159758501-0927596f-b332-4eb7-939b-8f2c95bf44d5.png)
Alternatively, you can remove the node from the list of nodes (which is what
cytoscapeobj.graph.remove_node
does internally) to the same effect:Inspect the list of nodes
JS console logs indicate an error immediately after running
cytoscapeobj.graph.remove_node_by_id('n3')
: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