-
Notifications
You must be signed in to change notification settings - Fork 533
Fixed write_dot error for networkx>=1.11 in nipype/pipeline/utils.py #1358
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
Conversation
@@ -1021,7 +1021,10 @@ def export_graph(graph_in, base_dir=None, show=False, use_execgraph=False, | |||
suffix='.dot', | |||
use_ext=False, | |||
newpath=base_dir) | |||
nx.write_dot(pklgraph, outfname) | |||
if float(nx.__version__)<1.11: |
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.
This won't work because, e.g., 1.9
is numerically greater than 1.11
.
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.
True, how dumb
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.
you can use: LooseVersion(nx.__version__) < LooseVersion('1.11')
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.
Is it necessary to do the version check? NX removed the top-level import, but did they move the function?
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.
I changed it to do the proper version check suggested by @satra
if LooseVersion(nx.__version__) < LooseVersion('1.11'): | ||
nx.write_dot(pklgraph, outfname) | ||
else: | ||
nx.drawing.nx_pydot.write_dot(pklgraph, outfname) |
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.
@mwaskom - it's possible that they changed the api here completely.
@polosecki - could you please check if a former version of nx has this function, in which case you don't need the `if ... else ...``` clause.
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.
It looks like the change just removed the direct import inside nx: networkx/networkx#1929
…, valid for all nx versions.
Now the change is done without checking nx version, since the difference with older nx version is only that they removed the direct import but the write_dot function was always defined in the same place. |
it seems backwards compatible with 1.10 |
Fixed write_dot error for networkx>=1.11 in nipype/pipeline/utils.py
This fixes an incompatibility with networkx>=1.11 reported here:
#1350 (comment)