Dagger.jl
Dagger.jl copied to clipboard
Fix write dag
Fixes for write_dag() and show_logs() functions. More details on #512 .
Awesome stuff! I'm happy to accept this as-is if you'd like, but there are also a few things we can improve if you want to tackle them:
- Add a
render_logsalternative (similar torender_logs(logs, :graphviz)) that returns a renderable GraphViz object (and equivalently, we could implementshow_logs(logs, :graphviz)like you've done here for rendering directly to anIO) - Consider adding a
save_logsfunction that drives Cairo to save renderings to a file, or makerender_logs(path::String, ...)somehow dispatch to do this (maybe via FileIO.jl) - Document (under
docs/src/logging-visualization.md) this newly-supported renderer, so that users can more easily discover it
Let me know if you want this merged now or if you want to try to tackle some of these, I can always add these to the TODO list :smile:
- Add a
render_logsalternative (similar torender_logs(logs, :graphviz)) that returns a renderable GraphViz object (and equivalently, we could implementshow_logs(logs, :graphviz)like you've done here for rendering directly to anIO)- Consider adding a
save_logsfunction that drives Cairo to save renderings to a file, or makerender_logs(path::String, ...)somehow dispatch to do this (maybe via FileIO.jl)- Document (under
docs/src/logging-visualization.md) this newly-supported renderer, so that users can more easily discover itLet me know if you want this merged now or if you want to try to tackle some of these, I can always add these to the TODO list 😄
Looks like it is not a problem to add this. I will do that soon
Hi!
There is an issue with implementing these features:
- Add a render_logs alternative (similar to render_logs(logs, :graphviz)) that returns a renderable GraphViz object (and equivalently, we could implement show_logs(logs, :graphviz) like you've done here for rendering directly to an IO)
- Consider adding a save_logs function that drives Cairo to save renderings to a file, or make render_logs(path::String, ...) somehow dispatch to do this (maybe via FileIO.jl)
It seems that GraphVizSimpleExt, which I am working on, was intended to be used when GraphViz is not imported (with GraphVizExt for the opposite case). Am I correct in this assumption?
If that is the case, GraphVizSimpleExt can not return a renderable GraphViz object, which we want from the render_logs function and which is to be used in save_logs.
Hi, I'm sorry about the long delay in my reply! You are welcome to add GraphViz as a dependency to GraphVizSimpleExt - I just didn't add it since I hadn't gotten the code really working, and it didn't currently depend on it. But it definitely seems like the right thing to do!