cytoscape / ipycytoscape

A Cytoscape Jupyter widget
https://ipycytoscape.readthedocs.io/en/master/
BSD 3-Clause "New" or "Revised" License
264 stars 62 forks source link

Parent / child relation is not rendered when child is added before parent #302

Open jenshnielsen opened 2 years ago

jenshnielsen commented 2 years ago

Bug report

Bug summary

Rendering of parent child relationship depends on the order of nodes being added

Code for reproduction

Consider the following CustomNode class which sets an optional parent child relationship

from typing import Optional

import ipycytoscape

class CustomNode(ipycytoscape.Node):
    """
    A node class that contains the correct information for visualization
    with ipycytoscape
    """

    def __init__(self, name: str, classes: str = "", parent: Optional[str] = None):
        super().__init__()
        self.data["id"] = name
        if parent is not None:
            self.data["parent"] = parent
        self.classes = classes

If I create a graph manually adding first the parent and then the child

graphwidget = ipycytoscape.CytoscapeWidget()
graphwidget.graph.add_nodes([CustomNode(name="node1", parent=None), CustomNode(name="node2", parent="node1")])
graphwidget

This correctly renders the parent child relation ship

image

However if I add the nodes in the reverse order:

graphwidget = ipycytoscape.CytoscapeWidget()
graphwidget.graph.add_nodes([CustomNode(name="node2", parent="node1"), CustomNode(name="node1", parent=None)])
graphwidget

The parent child relationship is not shown

image

Actual outcome

Parent child relationship is only shown when parent is added to the graph before child.

Expected outcome

Parent Child relationships is independent of the order of the nodes being added.

Version Info

jenshnielsen commented 2 years ago

Note: I tried reproducing this by modifying https://cytoscape.org/cytoscape.js-cola/demo-compound.html and here

        elements: [
        { group: 'nodes', data: { id: 'n1', parent: 'n0'} },
        { group: 'nodes', data: { id: 'n0' } },
        ]

renders identically to

        elements: [
        { group: 'nodes', data: { id: 'n0' } },
        { group: 'nodes', data: { id: 'n1', parent: 'n0'} },
        ]
ianhi commented 2 years ago

hmm that's no good. I wonder if this is an issue with what we're syncing (i.e. python side) or with the rendering (typescript side).

Can you try adding some parent relationships to this example: https://github.com/cytoscape/ipycytoscape/blob/master/examples/Text%20on%20node.ipynb

because there we will have a signal as to whether data is syncing properly via the name

jenshnielsen commented 2 years ago

Thanks, Yes I will start debugging it. It seems to me that there is some kind of sync issue with fields inside data as in thats how I discovered that https://github.com/cytoscape/ipycytoscape/pull/306 did not do classes like other notebooks.

Unlike the html callback notebook https://github.com/cytoscape/ipycytoscape/blob/master/examples/HTML%20interaction%20with%20graph.ipynb changing the class inside data did not trigger an update to the node style.

ianhi commented 2 years ago

I wonder if this is an issue with spectate or our usage of it. It sounds like you've run into other issues with syncing the data .And looking at out MutableDict https://github.com/cytoscape/ipycytoscape/blob/2ee745f0ea63be97283790f29bffda08d51cff48/ipycytoscape/cytoscape.py#L149-L173


Spectate is what lets us sync mutable data structures, which traitlets does not normally do.

it's defintiely a bit sparse compared to the most recent spectate + traitlets example: https://python-spectate.readthedocs.io/en/latest/usage/spectate-in-traitlets.html

jenshnielsen commented 2 years ago

@ianhi thanks for the hint. I will do some digging once I have a bit of time unless you get there first :)

ianhi commented 2 years ago

can you try adding the following to the end your custom node init:

self.send_state("data")
ianhi commented 2 years ago

trying to force a sync via this method: https://github.com/jupyter-widgets/ipywidgets/blob/11ade00777f1d27e2e1333694e7ae7b532ad791e/python/ipywidgets/ipywidgets/widgets/widget.py#L494-L501

jenshnielsen commented 2 years ago

Unfortunately that did not seem to resolve the issue. I will keep looking

marimeireles commented 2 years ago

So... Apparently, somehow this is related to the position of the nodes in the list. If I change the code for adding new nodes to something like the following:

    def add_nodes(self, nodes):
        """
        Appends nodes to the end of the list. Equivalent to Python's extend method.

        Parameters
        ----------
        nodes : list of ipycytoscape.Node
        """
        node_list = list()
        for node in nodes:
            if node.data["id"] not in self._adj:
                print('🐥')
                self._adj[node.data["id"]] = dict()
                node_list.insert(0, node)
                # node_list.append(node)
        self.nodes.extend(node_list)

Then the 2nd example works, but the first one doesn't.

What if we make some kind of sorting function for this? What do you think? Is this the way to go or am I missing something?

Might get really expensive though, to add nodes on the list, so if we go with this solution we have to think this through.

Thanks a lot for the great bug report and for catching this one @jenshnielsen.

jenshnielsen commented 2 years ago

@marimeireles Thanks thats interesting. I always assumed that this had something to do with the events not being triggered correctly on the ts/js side when inserting but your example clearly show that its not that but the order of the elements. Sorting could be one way. I originally found this with a bigger networkx graph which was actually build from presorted subgraphs (with parents before children) but for some reason when I merge it in networkx the order becomes reversed.

jenshnielsen commented 2 years ago

Digging a bit further. If I log the value of the NodeModel inside addNodeModel the value of parent is undefined if If I add the node with the parent attribute before adding the parent node but has the correct value if I do it the other way around

jenshnielsen commented 2 years ago

Ok so this is actually a limitation of the Cytoscape.js api.

If one does

cy.add({data: {id: "2"}})
cy.add({data: {id: "1",
             parent: "2"}});

directly in js on a cytoscape object this will render correctly but

cy.add({data: {id: "2"}})
cy.add({data: {id: "1",
             parent: "2"}});

I guess there are two ways this could be midigated in ipycytoscape We could sort the nodes as @marimeireles suggests as above or we could perhaps rework the code such that a call to add_nodes will result in a single call to add in the cytoscape.js api rather than adding them one by one

jenshnielsen commented 2 years ago

also note that changing the parent property via data is not actually supported in cytoscape.js after the graph has been created https://js.cytoscape.org/#notation/compound-nodes