Skip to content
This repository has been archived by the owner on Oct 21, 2021. It is now read-only.

Modify to_dot #206

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

clclcocoro
Copy link

Thank you for creating the amazing library.

I noticed that to_dot() function did not print the edge attributes in Graphs.graph(ExVertex[], ExEdge{ExVertex}[], is_directed=false).
I modified the corresponding line in src/dot.jl and added test codes to test/dot2.jl.

Would you please review these changes?

@mschauer
Copy link

to_dot shout also allow for preamble blocks as in

    graph [bgcolor=black]
    node [
        colorscheme="spectral10"
        shape=point
        ];
    edge [
        colorscheme="spectral10"
        ];

currently only isolated attributes=value pairs are allowed.

@clclcocoro
Copy link
Author

I understood.

@@ -30,7 +30,7 @@ function to_dot{G<:AbstractGraph}(graph::G, stream::IO,attrs::AttributeDict=Attr
write(stream,"$(vertex_index(vtx,graph))$attrs\n")
end
for edge in edges(graph)
write(stream,"$(vertex_index(source(edge), graph)) $(edge_op(graph)) $(vertex_index(target(edge), graph))\n")
write(stream,"$(vertex_index(source(edge), graph)) $(edge_op(graph)) $(vertex_index(target(edge), graph))$(has_edge_attrs ? string(" ", to_dot(attributes(edge, graph))) : "")\n")
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi, I'm not sure we should be calling to_dot inside the string interpolation here. Could you maybe just explain a little more on why calling to_dot(attributes(edge, graph))) inside the string interpolation is the right thing to do? Sorry, but I am not the most knowledgeable on to_dot. Although, I would expect a lower level functio that unpacks the graph edge attributes into a string -- is to_dot really the best one?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants