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

Add a demo notebook that demonstrates loading from and saving to neo4j #1184

Merged
merged 11 commits into from Apr 7, 2020

Conversation

huonw
Copy link
Member

@huonw huonw commented Mar 31, 2020

This adds a notebook to basics/ that demonstrates a few ways one might load graphs and subgraphs from Neo4j into memory (as Pandas DataFrames) and then convert those into StellarGraph objects. It follows basically the same structure as the Pandas and NetworkX ones, except has an additional "save results" section at the end too.

See: #1034 and #1037

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

You'll be able to see Jupyter notebook diff and discuss changes. Powered by ReviewNB.

@codeclimate
Copy link

codeclimate bot commented Mar 31, 2020

Code Climate has analyzed commit 4c5bffa and detected 0 issues on this pull request.

View more on Code Climate.

@huonw huonw changed the title Add a demo notebook that demonstrates loading from neo4j Add a demo notebook that demonstrates loading from and saving to neo4j Apr 1, 2020
@huonw huonw requested review from kjun9 and timpitman April 1, 2020 06:22
@huonw huonw marked this pull request as ready for review April 1, 2020 06:22
@@ -0,0 +1,2038 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use single Cypher query to retrieve an identifier for the source and target of each node is sufficient.

"We can use a single Cypher query to retrieve the identifiers of the source and target nodes of each edge."?

ensure the DataFrame columns have the easiest name.

"ensure the DataFrame columns match the defaults"?


Reply via ReviewNB

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, I really mangled that first sentence!

demos/basics/loading-saving-neo4j.ipynb Show resolved Hide resolved
@@ -0,0 +1,2038 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One way sort of a subgraph someone might be interested is one where the nodes and/or edges satisfy certain criteria.

"One type of subgraph someone might be interested..."?

Also in general, this Subgraphs sections feels more like a cypher tutorial to me, rather than anything stellargraph specific - I understand that it comes from us believing it would be a common use-case since you probably wouldn't load ALL your neo4j data into memory, but the exact way the user should reduce their graph is probably going to be very specific to their data. Thoughts?


Reply via ReviewNB

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ~whole notebook is a cypher tutorial, since it's actually not doing anything StellarGraph-y that isn't already explained in loading-pandas.ipynb. I agree that the exact reduction is specific to their data and their use case, but I feel that "filter the elements (in isolation)" and "k-hop subgraphs" are likely to be the most common (or at least, serve as close-enough examples) and so we can spell them out to get people started, and reduce the barriers to working with StellarGraph.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ~whole notebook is a cypher tutorial, since it's actually not doing anything StellarGraph-y that isn't already explained in loading-pandas.ipynb.

I guess so, but there's some relationship between the query being run and the way the stellargraph constructor is used that the notebook is demonstrating (e.g. run a query to obtain heterogeneous graph then use it to create a heterogeneous stellargraph), whereas for the subgraph one feels more like it's purely in cypher (i.e. stellargraph doesn't know/care that it's a subgraph). That said, I'm fine with keeping it since I agree it's likely to be a common use-case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I understand the distinction you were seeing, and it makes sense to me now. I'll leave it as is, but we can always rethink.

@huonw huonw requested a review from kjun9 April 6, 2020 02:57
Copy link
Contributor

@kjun9 kjun9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 👍

Copy link
Contributor

@timpitman timpitman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an excellent tutorial both for StellarGraph and Cypher! I do feel that the k-hop subgraph section goes beyond being a "basic" tutorial, however I think this notebook is the best place to put it for now.

Hopefully we can land #1183 before release to remove the FIXME's.

@huonw huonw merged commit 59d269a into develop Apr 7, 2020
@huonw huonw deleted the feature/1034-load-from-neo4j branch April 7, 2020 22:50
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

Successfully merging this pull request may close these issues.

None yet

3 participants