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

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

Open
jenshnielsen opened this issue Jan 28, 2022 · 13 comments

Comments

@jenshnielsen
Copy link
Contributor

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

  • ipycytoscape version (import ipycytoscape; print(ipycytoscape.__version__)) : 1.3.2
  • Python version: 3.8.12 and 3.10.0
  • Jupyter(Lab) version: jupyterlab 3.2.8
@jenshnielsen
Copy link
Contributor Author

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
Copy link
Collaborator

ianhi commented Feb 2, 2022

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
Copy link
Contributor Author

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 #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
Copy link
Collaborator

ianhi commented Feb 2, 2022

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

class Mutable(TraitType):
"""A base class for mutable traits using Spectate"""
_model_type = None
_event_type = "change"
def instance_init(self, obj):
default = self._model_type()
@mvc.view(default)
def callback(default, events):
change = dict(
new=getattr(obj, self.name),
name=self.name,
type=self._event_type,
)
obj.notify_change(change)
setattr(obj, self.name, default)
class MutableDict(Mutable):
"""A mutable dictionary trait"""
_model_type = mvc.Dict


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
Copy link
Contributor Author

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

@ianhi
Copy link
Collaborator

ianhi commented Feb 2, 2022

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

self.send_state("data")

@ianhi
Copy link
Collaborator

ianhi commented Feb 2, 2022

@jenshnielsen
Copy link
Contributor Author

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

@marimeireles
Copy link
Collaborator

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
Copy link
Contributor Author

@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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants