cytoscape / ipycytoscape

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

Convert the python graph to cytoscape graph #64

Open marimeireles opened 4 years ago

marimeireles commented 4 years ago

So, @ianhi PR https://github.com/QuantStack/ipycytoscape/pull/63 made me realized that not all attributes of a graph are being converted to a cytoscape graph.

We need to do the same thing for all attributes.

ianhi commented 4 years ago

Ohhh do you mean that all of these are valid in cytoscapejs? https://github.com/QuantStack/ipycytoscape/blob/8769ec2d36a9ae5ec8420ffc1c96fbb0f062d284/ipycytoscape/cytoscape.py#L84-L91

marimeireles commented 4 years ago

Uhum, and also these: https://github.com/QuantStack/ipycytoscape/blob/8769ec2d36a9ae5ec8420ffc1c96fbb0f062d284/ipycytoscape/cytoscape.py#L93-L94

Theoretically, everything that's documented here: https://js.cytoscape.org/#notation/elements-json is available in the frontend and just needs to be connected.

And the whole website actually. There are some really interesting functions like different ways to cluster nodes, etc. Lots of stuff to expand on :)

ianhi commented 4 years ago

Gotcha.

One other thing that might be nice to consider is that in cytoscapejs classes can be either a list of strings ['class1', 'class2'] or a string with classes separated by spaces class1 class2. On the python side it might be easier to work with lists of classes rather than a string. Maybe the objects could convert lists of strings to strings for the user? i.e.

if isinstance(classes, list):
   classes = classes.join(' ')

Since both typescript and traitlets introduce typing this would be easiest to handle before it makes it either of them.

MridulS commented 4 years ago

While being on the design choices for this, one more thing I think would improve the graph code would be to use a default graph data structure like adjacency list, instead of independent lists of nodes and edges. This makes it much easier to manipulate the graph data. Something like https://github.com/QuantStack/ipycytoscape/blob/8dbe4cf1ef64b089592e06a6492fbd33deadd86d/ipycytoscape/cytoscape.py#L113 , which in turn is a dict of a dict of a dict.

example

graph._adj = 
{ 'node1': {'node2': {edge attributes}},
'node2' : {'node1': {edge attributes}, 'node3': {edge attributes}}
'node3': {'node2': {edge attributes}}}

With these we can embed edge attributes (like style of a particular edge) into the graph data structure while communicating it with the frontend. I think currently we can only manipulate the graph wide style data.

Let me know what do you folks think about this

ianhi commented 4 years ago

(I didn't design this widget so take whatever I say with a grain of salt and check with @marimeireles for the truth)

@MridulS You're definitely right that an adjacency matrix would make manipulating the graph easier. However, I think that this library is not intended to be a graph manipulation library, but rather to serve as bridge between python and javascript for nice display. So after manipulating the graph with your library of choice you pass it to cytoscapejs via this widget:

networkx (manipulation) -> ipycytoscape (bridge) -> cytoscape.js (display)

If you look at the data structure used by cytoscapejs it's pretty much independent lists of nodes and edges. So its simpler to do the conversion to javascript when using independent lists rather than an adjacency matrix.

As for more granular style: cytoscapejs actually provides this by allowing any given node or edge to have an arbitrary number of classes (a la css). More generally the attributes listed in the first few posts of this issue are the only ones that are interpretable by cytoscapejs. So ideally the graph creation methods would intelligently populate those fields of the Nodes and Edge objects. For example searching through the data attribute of a networkx node for attributes like classes, selectable etc...

ianhi commented 4 years ago

FWIW I think properly doing the conversion from something like networkx attributes to cytoscape attribute would also solve some other issues. For example it would solve https://github.com/QuantStack/ipycytoscape/issues/45#issuecomment-623363722

That's a fix, but maybe better solution would be to try to fit networkx class attributes in a cytoscape object. So, if you have keywords in a networkx graph that match the keywords existent in a cytoscape graph you could make a better conversion from one to another. And the keywords that don't match could be just added to data.

MridulS commented 4 years ago

On 08-May-2020, at 9:47 AM, Ian Hunt-Isaak notifications@github.com wrote:

(I didn't design this widget so take whatever I say with a grain of salt and check with @marimeireles https://github.com/marimeireles for the truth)

@MridulS https://github.com/MridulS You're definitely right that an adjacency matrix would make manipulating the graph easier. However, I think that this library is not intended to be a graph manipulation library, but rather to serve as bridge between python and javascript for nice display.

(Just my opinions, could be wrong about pretty much everything :) )

True this isn’t a graph manipulation library but my reasoning behind keeping a (hidden from user) graph data structure is to make testing, bug fighting and having a single source of truth. For example, successive add/remove_edge/node/graph, method has to keep track of both nodes and edges and make sure both the lists are updated properly with their attributes. By using an “internal” graph data structure a bunch of potential bugs can be avoided (like the clear graph one). Every graph object edit method can end with something like self.sync_updates() which syncs the graph data dict with the list of nodes and edges cytoscape objects, which goes to the frontend.

Thoughts?

So after manipulating the graph with your library of choice you pass it to cytoscapejs via this widget:

networkx (manipulation) -> ipycytoscape (bridge) -> cytoscape.js (display) If you look at the data structure used by cytoscapejs it's pretty much independent lists of nodes and edges. So its simpler to do the conversion to javascript when using independent lists rather than an adjacency matrix.

As for more granular style: cytoscapejs actually provides this by allowing any given node or edge to have an arbitrary number of classes (a la css). More generally the attributes listed in the first few posts of this issue are the only ones that are interpretable by cytoscapejs. So ideally the graph creation methods would intelligently populate those fields of the Nodes and Edge objects. For example searching through the data attribute of a networkx node for attributes like classes, selectable etc...

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/QuantStack/ipycytoscape/issues/64#issuecomment-625620837, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABI5RFBXUBUWBH5SOAZEEELRQOBUNANCNFSM4MZP5MWA.

MridulS commented 4 years ago

FWIW I think properly doing the conversion from something like networkx attributes to cytoscape attribute would also solve some other issues. For example it would solve #45 (comment)

+1

timkpaine commented 4 years ago

@MridulS thats how ipydagred3 does it, all manipulations end in a "sync" method and I keep a dict or arbitrary attrs so not every dagre attribute has to map back to an explicit python type https://github.com/timkpaine/ipydagred3/blob/master/ipydagred3/edge.py#L26

MridulS commented 4 years ago

@timkpaine thanks for the link! yeah that's exactly what I had in mind.

ianhi commented 4 years ago

@marimeireles I had a go at this and got on stuck what to do in the on:change method on the typescript side. These attributes being changed should trigger a re-render. But currently the only way to cause a re-render is to call init_render in such a way that a new cytoscape object is created: https://github.com/QuantStack/ipycytoscape/blob/fdb8ea14d93b55ebb56a2d4ea286ab152ed6f96a/src/widget.ts#L348-L358

I'm not sure how heavy this actually is but I don't think its optimal. Is it worth looking into: https://js.cytoscape.org/#collection/graph-manipulation which are ways of modifying these traits that will result in a re-render, but not a total recreation of the graph. Probably also nice to explore: https://js.cytoscape.org/#cy.batch

Also it looks like the NodeViews class has the beginnings of managing this. But it is never actually used to make the graph. Is this object vestigal? Can it be wrapped into the Node class? https://github.com/QuantStack/ipycytoscape/blob/fdb8ea14d93b55ebb56a2d4ea286ab152ed6f96a/src/widget.ts#L185

ianhi commented 4 years ago

This maybe ought to be re-opened. I think that for JSON or networkx this works, but not for dataframes yet. They need to have a dataframe version of this function used for networkx: https://github.com/QuantStack/ipycytoscape/blob/0495c5f1721928ca9877cdc0498774eaac74f5be/ipycytoscape/cytoscape.py#L202-L207

joseberlines commented 4 years ago

@ianhi, thanks for open. Jsons are of course typical python data but it is not feasible to work with data in json form for calculations etc, so once df is there it would be straight forward to create a graph. Perhaps the way around is create a networkx from the DF and then the plot via networkx. but there again I don't know how many atributes are taken from the networkx. Actually I guess networkx does not include layout attributes. So we are in the same case using Networkx as using pandas.

marimeireles commented 4 years ago

For one to fix this issue they basically have to copy paste the code pointed by Ian here: https://github.com/QuantStack/ipycytoscape/issues/64#issuecomment-645064201