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

Replace pygraphviz with graphviz #80

Merged
merged 15 commits into from
Sep 13, 2023
Merged

Conversation

chensgit169
Copy link
Collaborator

Considering python package graphviz is more lightweight, installation-flexible, well-documented and simple to use, implemented draw_dag() with graphviz and removed pygraphviz. Dependence upon IPython was also removed since this is not generally compatiable with different ways to run pyquafu. Instead supports for notebook users should be tested and documented independently in the later development -- luckily, it seems `graphviz`` does well in the aspect.

Copy link
Collaborator

@Zhaoyilunnn Zhaoyilunnn left a 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
Copy link
Collaborator

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.

Copy link
Collaborator Author

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?

Copy link
Collaborator

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.

quafu/visualisation/draw_dag.py Outdated Show resolved Hide resolved
@Zhaoyilunnn Zhaoyilunnn merged commit d6ea733 into ScQ-Cloud:master Sep 13, 2023
48 checks passed
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.

2 participants