-
Notifications
You must be signed in to change notification settings - Fork 27
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
Added a version check before calling http.socket_timeout #32
Conversation
…j.py compatible with newer versions of py2neo Modified travis config to test both py2neo 3 and 4
At this point it may make sense to also move the py2neo imports out the top-level module so we don't burden users with this dependency unless they need it. Perhaps tqdm as well. |
…j.py compatible with newer versions of py2neo Modified travis config to test both py2neo 3 and 4 Moved the py2neo import logic into a function called when needed to keep the other functions from having an unnecessary dependency Modified construct_pdp_query to run more efficiently and return a string representation of the paths. Modified tests to handle the new output Added tests to check whether the queries produced by construct_pdp_query are formatted correctly Removed multiple py2neo versions from .travis.yml
Pulling from hetio/hetio
Sorry for the commit naming weirdness, I squashed too many commits together, which caused me to have to merge with upstream despite not having new changes there. Is it better to leave all the commits as individuals when creating the pull request, then squash them at the end? |
I reverted my master branch to only contain the changes related to py2neo, and added the rest of the requested changes in the latest pull request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In future, don't make changes to your master branch (only fast forward your master branch to stay up to date with upstream:master
. However, for this PR, whose head is your master branch, let's finish it up and merge. Then we can fix your branches.
hetio/neo4j.py
Outdated
""" | ||
import py2neo | ||
# Get py2neo version | ||
PY2NEO_VER = int(py2neo.__version__[0]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we do PY2NEO_VER = tuple(int(x) for x in py2neo.__version__.split('.'))
. Then if PY2NEO_VER < (4, 0)
. This is more resilient if major version number becomes double digits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually it's probably best to just use
import pkg_resources
version = pkg_resources.parse_version(py2neo.__version__)
if version._version.release[0] < 4:
# blah
https://setuptools.readthedocs.io/en/latest/pkg_resources.html#parsing-utilities
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And perhaps import_py2neo should return py2neo, version_tuple
where version_tuple is version._version.release
. Then this can be used in other functions.
hetio/neo4j.py
Outdated
@@ -165,7 +179,6 @@ def cypher_path(metarels): | |||
def construct_dwpc_query(metarels, property='name', join_hint='midpoint', index_hint=False, unique_nodes=True): | |||
""" | |||
Create a cypher query for computing the *DWPC* for a type of path. | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want to just save the whitespace changes for the other PR that modifies the PDP / DWPC functions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure thing
… the version tuple
Added the requested changes, sorry that I missed the whitespace changes in git diff earlier |
@ben-heil great. So now I've merged this, you should configure |
That makes sense, will do |
Addressing #31
This should make it possible to import neo4j.py regardless of which py2neo version you're using. I looked at the documentation for version 4 and the functions that actually use py2neo should still work fine. I'd write tests for them to make sure that's the case, but they all modify or delete information in existing neo4j databases. It should be possible to spin up a neo4j database at test time if we want to take that route.