This repository has been archived by the owner on Jul 3, 2023. It is now read-only.
Stefan/refactor visualization #58
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
skrawcz
force-pushed
the
stefan/refactor_visualization
branch
from
380c8a5
to
b87c8bc
Compare
Visualization was an afterthought we added quickly. This change takes in feedback from #45 and makes it more amenable for actual use. We therefore expose new functions, and deprecate the `visualize` parameter in `driver.execute()`. The new functions enable the user to pass in a dictionary that will be used as key word arguments (kwargs) to the render function -- thereby giving lots of control on how things are visualized. Along the way I also refactored the cycle detection using networkx. This again, was a quick hack, now it's a standalone implementation. We do break graph behavior, but not driver behavior with this refactor. I believe that's fine, since we don't want people to depend on FunctionGraph directly just yet. Adds unit tests for these changes.
This is to keep it consistent with the visualize_execution() function. This change is backwards compatible, so no changes on the user required.
Since we deprecated display=True in execute, this change shows how one would visualize the DAG with the proper function calls on the driver that now exist.
So that way we don't waste time trying to install it on python 3.7+ as it's in the standard library then.
skrawcz
force-pushed
the
stefan/refactor_visualization
branch
from
b87c8bc
to
0046cae
Compare
Graphviz needed to be installed for things to work. Ugh. Bloat, but oh well.
Due to: 1. we don't support any URI. 2. it is now consistent with language in graph.py.
It's a list of list of strings. where the strings are node names that form a cycle.
Neither graph libraries output a stable format for some reason. So explaining that in the comment.
elijahbenizzy
approved these changes
Feb 6, 2022
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Visualization was an afterthought we added quickly.
This change takes in feedback from #45 and makes
it more amenable for actual use.
We therefore expose new functions, and deprecate
the
visualizeparameter indriver.execute().The new functions enable the user to pass in a dictionary that will
be used as key word arguments (kwargs) to the
render function -- thereby giving lots of control on
how things are visualized. We also mirror the changes
in display_all_functions().
Along the way I also refactored the cycle detection
using networkx. This again, was a quick hack,
now it's a standalone implementation. We do break graph
behavior, but not driver behavior with this refactor. I
believe that's fine, since we don't want people to
depend on FunctionGraph directly just yet.
Adds unit tests for these changes.
Additions
Removals
Changes
Testing
Checklist
Testing checklist
Python
Loosely tested across different python versions.