cytoscape / ipycytoscape

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

Proposal for an add_graph_from_neo4j pull request #226

Closed pwrose closed 3 years ago

pwrose commented 3 years ago

Problem

There is currently no solution to display Neo4j graphs or the result of a Neo4j Cypher query (subgraph) in Jupyter Notebook.

Proposed Solution

I can make a pull request for aadd_graph_from_neo4jmethod that takes as input a Neo4j graph or subgraph.

Additional context

This method would add a dependency on the py2neo library. Let me know if that's ok.

marimeireles commented 3 years ago

Hey @pwrose I wrote a notebook with examples integrating ipycytoscape with neo4j a while ago when we worked together in that COVID effort, do you still have access to it? I think they can be very useful for what you want to do. I think it's fine to add py2neo as a dependency, we added pandas and networkx. I don't think size of the binary is a huge issue for our users. @ianhi has an opinion on this? Thank you very much :)

pwrose commented 3 years ago

@marimeireles I've created a prototype of the Neo4j integration here: https://github.com/pwrose/ipycytoscape/blob/master/ipycytoscape/cytoscape.py#L642

I need to do some more testing before submitting a pull request.

I've created a couple of example notebooks to demonstrate how it works: https://github.com/pwrose/neo4j-ipycytoscape

You can run these examples on the Pangeo Binder server here: https://binder.pangeo.io/v2/gh/pwrose/neo4j-ipycytoscape/master

There are a couple of problems with providing runnable examples on MyBinder.org:

So I'm not sure what is the best option to provide a runnable example for the ipycytoscape repo.

marimeireles commented 3 years ago

I just skimmed your code, but it seems like you're in a really good path! Exciting work :)

Notebook Neo4j_Example2.ipynb is a more comprehensive example that uses our COVID-19-Net Neo4j instance. It works fine on the Pangeo Binder, but doesn't work on MyBinder.org. I'm not sure why, e.g., a blocked port?

I don't think it's necessary to run it on MyBinder, but if you want help to debug it let me know.

So I'm not sure what is the best option to provide a runnable example for the ipycytoscape repo.

It's not mandatory to have an example, but I don't think we can't run two different binders in the same repo.

Maybe we could just link to your repo? I'd prefer to use a different repo than integrating your changes here because I don't want to waste resources, if we don't have to load pangeo stuff every time someone opens ipycytoscape binder I don't think we should.

https://github.com/pwrose/neo4j-ipycytoscape/blob/77ab1b42b2176ade78a733b73c83a421de7f30cb/binder/postBuild#L19

btw I think there's an issue here, the version of ipycytoscape is fixed to a really old one in this file.

MridulS commented 3 years ago

Notebook Neo4j_Example2.ipynb is a more comprehensive example that uses our COVID-19-Net Neo4j instance. It works fine on the Pangeo Binder, but doesn't work on MyBinder.org. I'm not sure why, e.g., a blocked port?

Yeah, only certain ports are allowed for egress traffic on mybinder:

https://github.com/jupyterhub/mybinder.org-deploy/blob/a11fe1d3ad104c3a4777babf586b1f73ec08e8da/mybinder/values.yaml#L44-L51

marimeireles commented 3 years ago

Hey @pwrose, let me know if I can help on this! I think it's a super cool addition. Cheers!

pwrose commented 3 years ago

@marimeireles my pull request (https://github.com/QuantStack/ipycytoscape/pull/237) fails with a lint issue. I'm not sure what needs to be fixed.

pwrose commented 3 years ago

@marimeireles the tests are also trying to run Neo4j_Example.ipynb on binder, however, this will not work since binder blocks required ports. Can you exclude Neo4j_Example.ipynb from running on binder?

marimeireles commented 3 years ago

@marimeireles my pull request (#237) fails with a lint issue. I'm not sure what needs to be fixed.

I'll have a look at this right now. Thanks for the awesome work! I'll move the rest of the discussion to the PR :)

ianhi commented 3 years ago

I think it's fine to add py2neo as a dependency, we added pandas and networkx. I don't think size of the binary is a huge issue for our users. @ianhi has an opinion on this?

It's certainly no worse than pandas or networkx. But reflecting on this more maybe we shouldn't be requiring any of them. It's nonsensical to call from_networkx without already having a networkx object which would imply having networkx installed. So I propose adding them as a optional dependencies so the install options would be:

# No extra graph libraries
pip install ipycytoscape

# just networkx
pip install ipycytoscape[networkx]

# with pandas
pip install ipycytoscape[dataframe]

# something that pulls in all optional deps
pip install ipycytoscape[all]

Then we should just lazy import the respective libraries in their from_____ functions.

marimeireles commented 3 years ago

Thanks for the great work @pwrose!