Add compatibility to work with py2neo version 3.1.2 #6

veleritas · 2017-02-15T19:14:21Z

Hi Daniel,

I've finished making my changes to the hetio package to make it compatible with py2neo version 3.1.2. I didn't actually have to end up changing too much in the package. The only real difference between py2neo versions is that when nodes and edges are added to neo4j, they are done as a single subgraph object, and not as multiple objects.

I also added a progress bar to the neo4j export process, which allows me to watch how things are going from the command line. This option is off by default in order to ensure it does not break your previous work. In that regard I have also tested things with your original code, and things worked fine.

I have only modified the neo4j.py file, and found that no other changes in hetio were needed to make the rest of the pipeline work. All other changes were done in the respective integrate and learn repositories.

One thing you might notice is that the uri variable passed to export_neo4j() is actually expected to be a py2neo.Graph object, but only if using py2neo version 3. In my testing it seemed that I could not get the py2neo URI resolver to work with the new Bolt protocol. Instead I had to open my own py2neo connection explicitly, and that seems to work. I believe the problem is that both the Bolt and HTTP ports have to be open, and on different ports, and the URI resolver might not support doing that.

Let me know if you have any questions!

Toby

dhimmel

Thanks for the contribution! I like the elegance of your code. I requested some changes, but overall this is a great enhancement to hetio.

I also added a progress bar to the neo4j export process, which allows me to watch how things are going from the command line. This option is off by default in order to ensure it does not break your previous work.

Cool that progress bar will be really helpful!

dhimmel · 2017-02-15T19:22:08Z

hetio/neo4j.py

@@ -71,7 +101,13 @@ def append(self, x):
    def create(self):
        if not self:
            return
-        self.db_graph.create(*self)
+
+# http://stackoverflow.com/questions/37676731/typeerror-create-takes-2-positional-arguments-but-4-were-given


dhimmel · 2017-02-15T19:23:29Z

hetio/neo4j.py

+
+# http://stackoverflow.com/questions/37676731/typeerror-create-takes-2-positional-arguments-but-4-were-given
+        if PY2NEO_VER == 3:
+            self.db_graph.create(union(self))


Let's put

union = lambda v: reduce(or_, v)

under this if statement, since it's not really needed anywhere else

dhimmel · 2017-02-15T19:29:23Z

hetio/neo4j.py

+
+    queue = graph.get_nodes()
+    if show_progress:
+        total_nodes = hetio.stats.get_metanode_df(graph)["nodes"].sum()


Let's try to remove the hetio.stats import:

total_nodes = len(graph.node_dict)

dhimmel · 2017-02-15T19:36:43Z

hetio/neo4j.py

+
+    queue = graph.get_edges(exclude_inverts=True)
+    if show_progress:
+        total_edges = hetio.stats.get_metaedge_df(graph)["edges"].sum()


Could wrap graph.get_edges(exclude_inverts=True) in list(). Then you could do total_edges = len(queue). I don't think it's likely that this list will push us over a memory limit in any real setting.

On second thought, perhaps I should add an n_nodes and n_edges attribute to graphs, which get updated when nodes or edges are created. What do you prefer?

In 903a4c8, I added n_nodes and n_edges as graph attributes. Therefore, you should be able to do total_edges = graph.n_edges (and similar for nodes).

You would have to fetch the upstream changes and rebase your branch on them. Then force push. Do you know how to do that?

Sure, I will figure it out.

dhimmel · 2017-02-15T19:37:23Z

setup.py

-with open('README.rst') as read_file:
-    long_description = read_file.read()
+#with open('README.rst') as read_file:
+#    long_description = read_file.read()


What's the reason for commenting this out?

Ah README.rst wasn't part of the repo. Added it, if you rebase, you should be good.

Don't worry about these commented lines. See #7. Will delete them after we merge this PR to avoid conflicts.

Yeah doing a pip install -e . for the repo was giving me an error with README.rst, so I just commented it out to get it to install correctly.

dhimmel · 2017-02-15T19:41:03Z

hetio/neo4j.py

@@ -8,14 +8,28 @@
 import py2neo.packages.httpstream
 import pandas

+from tqdm import tqdm
+from functools import reduce


The functools and operator imports should go in the first group of standard library imports. tqdm import should go with second group (third party imports). See https://www.python.org/dev/peps/pep-0008/#imports.

dhimmel · 2017-02-15T19:44:20Z

hetio/neo4j.py

-        self.db_graph.create(*self)
+
+# http://stackoverflow.com/questions/37676731/typeerror-create-takes-2-positional-arguments-but-4-were-given
+        if PY2NEO_VER == 3:


Perhaps if PY2NEO_VER >= 3 makes a little more sense, assuming v4 would be more like 3 than 2

dhimmel · 2017-02-15T19:53:29Z

hetio/neo4j.py

    """Export hetnet to neo4j"""
-    db_graph = py2neo.Graph(uri)
+
+    if PY2NEO_VER == 2:


Can we change the if statement to evaluate isinstance(uri, py2neo.Graph) rather than to evaluate the version? This way for py2neo 2 you could pass a py2neo.Graph. And I'm hopeful that https://github.com/technige/py2neo/issues/605 will eventually be fixed.

Sure, that's an easy fix I can make. I guess it might just confuse other people if they try to use the py2neo 3 URI resolver with bolt and have difficulties with getting it working.

dhimmel · 2017-02-27T21:23:46Z

@veleritas I'd love to get your contributions merged. Let me know if you want me to pick up where you've left off on this PR. But if you have the time, I think everything left to do should be pretty quick to implement.

veleritas · 2017-03-01T18:36:20Z

Yes, sorry, I've been busy, but will implement the changes you mentioned this week.

Tqdm progress bars will be used to monitor Neo4j network import progress.

Add tqdm progress bars to the node and edge importing steps.

Closes #4 Closes #8 Closes #6

dhimmel · 2017-03-03T01:52:42Z

@veleritas this is available in v0.2.2, but I'd recommend v0.2.3 which fixes a PyPI formatting issue.

Thanks for the contribution, you're now an author on the Zenodo https://zenodo.org/record/345182

dhimmel reviewed Feb 15, 2017

View reviewed changes

This was referenced Feb 15, 2017

Speeding up the Neo4j import #5

Open

Use setup.cfg to use README.md for PyPI description #7

Closed

Tong Shu Li added 2 commits March 2, 2017 19:22

Add tqdm as a dependency

82e1c43

Tqdm progress bars will be used to monitor Neo4j network import progress.

Add progress bars to neo4j importer

35ba28f

Add tqdm progress bars to the node and edge importing steps.

veleritas force-pushed the py2neo3 branch from 13416d9 to 35ba28f Compare March 2, 2017 19:59

veleritas closed this Mar 2, 2017

dhimmel mentioned this pull request Mar 2, 2017

Add py2neo version 3 URI compatibility #8

Merged

dhimmel pushed a commit that referenced this pull request Mar 2, 2017

Add py2neo version 3 URI compatibility (#8)

7fe01f1

Closes #4 Closes #8 Closes #6

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add compatibility to work with py2neo version 3.1.2 #6

Add compatibility to work with py2neo version 3.1.2 #6

veleritas commented Feb 15, 2017

dhimmel left a comment

dhimmel Feb 15, 2017

dhimmel Feb 15, 2017

dhimmel Feb 15, 2017

dhimmel Feb 15, 2017

dhimmel Feb 15, 2017

veleritas Feb 17, 2017

dhimmel Feb 15, 2017

dhimmel Feb 15, 2017

dhimmel Feb 15, 2017

veleritas Feb 17, 2017

dhimmel Feb 15, 2017

dhimmel Feb 15, 2017

dhimmel Feb 15, 2017

veleritas Feb 17, 2017

dhimmel commented Feb 27, 2017

veleritas commented Mar 1, 2017

dhimmel commented Mar 3, 2017

Add compatibility to work with py2neo version 3.1.2 #6

Add compatibility to work with py2neo version 3.1.2 #6

Conversation

veleritas commented Feb 15, 2017

dhimmel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dhimmel commented Feb 27, 2017

veleritas commented Mar 1, 2017

dhimmel commented Mar 3, 2017