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

Remove temporary DOT files #884

Merged
merged 3 commits into from
May 5, 2024
Merged

Remove temporary DOT files #884

merged 3 commits into from
May 5, 2024

Conversation

zilto
Copy link
Collaborator

@zilto zilto commented May 3, 2024

Hamilton visualizations rely on the graphviz library.
It defines graphs using the DOT language, which
defines one statement per line using a string.

Previously, Hamilton used graphviz.Digraph.render()
to produce visualizations. This has the side-effect
of producing an intermediary DOT file on disk. This
is most often of no use and clutters the directory.

Now, we are switching to graphviz.Digraph.pipe()
to write bytes directly to an open file. Tests were
updated accordingly.

The keyword argument keep_dot was added to viz
functions in case users still want this DOT file
to be produced. It allows to rerender the viz with
a different style without re-executing the Hamilton
code. It could be useful when iterating over custom
styling.

How I tested this

  • updated tests and they succeed

zilto added 2 commits May 3, 2024 19:32
Hamilton visualizations rely on the graphviz library.
It defines graphs using the DOT language, which
defines one statement per line using a string.

Previously, Hamilton used `graphviz.Digraph.render()`
to produce visualizations. This has the side-effect
of producing an intermediary DOT file on disk. This
is most often of no use and clutters the directory.

Now, we are switching to `graphviz.Digraph.pipe()`
to write bytes directly to an open file. Tests were
updated accordingly.

The keyword argument `keep_dot` was added to viz
functions in case users still want this DOT file
to be produced. It allows to rerender the viz with
a different style without re-executing the Hamilton
code. It could be useful when iterating over custom
styling.
@zilto zilto changed the title fix Remove temporary DOT files Remove temporary DOT files May 3, 2024
@zilto zilto added the enhancement New feature or request label May 3, 2024
@@ -894,7 +899,7 @@ def display(
custom_style_function=custom_style_function,
config=config,
)
kwargs = {"view": False, "format": "png"} # default format = png
kwargs = {"format": "png"} # default format = png
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why drop "view": False?

Copy link
Collaborator Author

@zilto zilto May 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

graphviz.Digraph.pipe() doesn't have a view kwarg. Forgot to add it back for keep_dot=True though.

@skrawcz skrawcz merged commit 54b2a4f into main May 5, 2024
23 checks passed
@skrawcz skrawcz deleted the fix/remove-tmp-dot branch May 5, 2024 22:38
skrawcz added a commit that referenced this pull request May 5, 2024
skrawcz added a commit that referenced this pull request May 5, 2024
This reverts commit 54b2a4f.
Need to debug it more before merging back in.
@zilto zilto mentioned this pull request May 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants