-
Notifications
You must be signed in to change notification settings - Fork 22
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
Replace pygraphviz
with graphviz
#80
Conversation
Decouple from IPython, pygraphviz
Though the newest version is 0.20.1, 0.14.1 works fine already.
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.
Seems that graphviz
works well, there are just two minor comments for your reference
@@ -253,9 +253,7 @@ def send(self, | |||
raise UserError() | |||
else: | |||
res_dict = response.json() | |||
import pprint |
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 assume that you want to use the verbose flag to control whether we print response, so you finally decide to remove this now? If so, maybe the verbose flag should be deleted.
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 I intended to help debug by setting verbose=True
, yet it was not completed. Let's delete it for now. By the way I just noticed status_code in two level in master branch were not seperated yet?
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 believe this is a mistake I made when I merge stable/0.3 branch into master. When resolving conflicts, I mistakenly removed the change in stable/0.3 branch and kept the master branch change. I will fix this later.
Considering python package
graphviz
is more lightweight, installation-flexible, well-documented and simple to use, implemented draw_dag() withgraphviz
and removedpygraphviz
. Dependence uponIPython
was also removed since this is not generally compatiable with different ways to runpyquafu
. Instead supports for notebook users should be tested and documented independently in the later development -- luckily, it seems `graphviz`` does well in the aspect.