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

cluster to cluster edge fails #89

Closed
GoogleCodeExporter opened this issue Apr 13, 2015 · 3 comments · Fixed by #238
Closed

cluster to cluster edge fails #89

GoogleCodeExporter opened this issue Apr 13, 2015 · 3 comments · Fixed by #238

Comments

@GoogleCodeExporter
Copy link

GoogleCodeExporter commented Apr 13, 2015

What steps will reproduce the problem?
To see if I'm it's possible to get the dependency graph from VCL/Delphi
project, I'm trying to reproduce this example
http://www.graphviz.org/content/fdpclust (directed)

everything works well, until it come to add edges on clusters then it fail.
(e-> clusterB and clusterC -> clusterD)

What version of the product are you using? On what operating system?
pydot 1.0.28 (patched so it find Graphviz in "Program Files (x86), python 3.3,
Windows 7 64 bits.

Joined : the function I use to produce the graph,

Original issue reported on code.google.com by pierre.p...@gmail.com on 7 Mar 2014 at 10:41

Attachments:

def GraphDirectedCluster():
    graph = pydot.Dot(graph_type='digraph')

    e = pydot.Node("e")
    graph.add_node(e)

    clusterA = pydot.Cluster("A")
    a = pydot.Node("a")
    b = pydot.Node("b")
    clusterA.add_node(a)
    clusterA.add_node(b)
    clusterA.add_edge(pydot.Edge(a,b))
    graph.add_subgraph(clusterA)`
    clusterC = pydot.Cluster("C")
    C = pydot.Node("C")
    D = pydot.Node("D")
    clusterC.add_node(C)
    clusterC.add_node(D)
    clusterC.add_edge(pydot.Edge(C,D))
    clusterA.add_subgraph(clusterC)

    clusterB = pydot.Cluster("B")
    d = pydot.Node("d")
    f = pydot.Node("f")
    clusterB.add_node(d)
    clusterB.add_node(f)
    clusterB.add_edge(pydot.Edge(d,f))
    graph.add_subgraph(clusterB)
    graph.add_edge(pydot.Edge(d,D))

    graph.add_edge(pydot.Edge(e,clusterB))
    graph.add_edge(pydot.Edge(clusterC,clusterB))

    print(graph.to_string())
    graph.write_png('directedGraphCluster.png')
@GoogleCodeExporter
Copy link
Author

I forgot : I use the pydot python3-branch.

Original comment by pierre.p...@gmail.com on 7 Mar 2014 at 10:47

@peternowee peternowee added the bug label Sep 14, 2020
@peternowee peternowee added this to the 1.4.2 milestone Sep 14, 2020
peternowee added a commit to peternowee/pydot that referenced this issue Sep 14, 2020
When instatiating a new `Edge`, if the caller supplies a `Subgraph` or
`Cluster` object as an edge point, store only the name string of that
object, same as we do for `Node` objects as edge points.

According to the DOT Language definition ([1]) an edge point may be a
node or a subgraph. Clusters are just subgraphs whose names begin with
`cluster`, so those are also allowed.

Although not documented, the `Edge` constructor already accepts
subgraphs and clusters as edge points as long as they are in the form
of name strings. These strings are stored in `Edge.obj_dict['points']`
and get included in the writing of a DOT string (`Edge.to_string()`),
just as name strings of nodes would.

However, until now, this is different when an edge point is supplied as
an object. For a `Node`, the name string gets looked up and stored in
`Edge.obj_dict['points']`, but `Subgraph` and `Cluster` objects are
stored there directly as objects.

This leads to problems when `Edge.to_string()` tries to concatenate the
different parts of the edge, with errors like:

    TypeError: unsupported operand type(s) for +: 'Subgraph' and 'str'

    TypeError: sequence item 2: expected str instance, Cluster found

I did not find any explanation for the difference, nor any code that
depends on the points being stored as objects.

The alternative of storing objects across the line was not chosen,
because it would have much larger implications both for the pydot code
itself as for user code accessing `Edge.obj_dict['points']` directly.

Fixes pydot#89.

[1]: https://www.graphviz.org/doc/info/lang.html
peternowee added a commit to peternowee/pydot that referenced this issue Sep 14, 2020
When instantiating a new `Edge`, if the caller supplies a `Subgraph` or
`Cluster` object as an edge point, store only the name string of that
object, same as we do for `Node` objects as edge points.

According to the DOT Language definition ([1]) an edge point may be a
node or a subgraph. Clusters are just subgraphs whose names begin with
`cluster`, so those are also allowed.

Although not documented, the `Edge` constructor already accepts
subgraphs and clusters as edge points as long as they are in the form
of name strings. These strings are stored in `Edge.obj_dict['points']`
and get included in the writing of a DOT string (`Edge.to_string()`),
just as name strings of nodes would.

However, until now, this is different when an edge point is supplied as
an object. For a `Node`, the name string gets looked up and stored in
`Edge.obj_dict['points']`, but `Subgraph` and `Cluster` objects are
stored there directly as objects.

This leads to problems when `Edge.to_string()` tries to concatenate the
different parts of the edge, with errors like:

    TypeError: unsupported operand type(s) for +: 'Subgraph' and 'str'

    TypeError: sequence item 2: expected str instance, Cluster found

I did not find any explanation for the difference, nor any code that
depends on the points being stored as objects.

The alternative of storing objects across the line was not chosen,
because it would have much larger implications both for the pydot code
itself as for user code accessing `Edge.obj_dict['points']` directly.

Fixes pydot#89.

[1]: https://www.graphviz.org/doc/info/lang.html
@peternowee
Copy link
Member

I ran the example code, which gave me the following error in Python 3:

Traceback (most recent call last):
  File "mre01a.py", line 35, in <module>
    print(graph.to_string())
  File "pydot.py", line 1564, in to_string
    graph.append( edge.to_string() + '\n' )
  File "pydot.py", line 875, in to_string
    return ' '.join(edge) + ';'
TypeError: sequence item 2: expected str instance, Cluster found

In Python 2, the error is:

TypeError: sequence item 2: expected string, Cluster found

The error complains about one of the edge points being a Cluster object instead of a string. Stepping through the code using pdb, I found the reason to be in the Edge constructor, where for Node object edge points the name string is looked up and stored, while for Subgraph and Cluster objects this is not done and they are just stored as-is, leading to the error.

I did not find any explanation for this difference. The DOT Language definition defines that both nodes and subgraphs can be points of an edge. A cluster is just a specific kind of subgraph, as also described there.

The reason this bug does not seem to get run into very often is, I think, twofold:

  1. Edges are probably more commonly set between nodes than between subgraphs or clusters. Notice that the pydot docstring for Edge also only mentions nodes as possible end points.
  2. Edges may be more often instantiated using the name strings of the edge points rather than the edge point objects. The OP's example code works when using name strings to refer to the edge points: graph.add_edge(pydot.Edge('e','cluster_B')).

Anyway, I have prepared PR #238 to remove this difference by always storing Subgraph and Cluster edge points as name strings, just like we do with Node edge points. I checked that it allows the example code of the OP to run successfully.

Some further remarks about the example code:

  • New link to the Graphviz example the OP mentioned: https://www.graphviz.org/Gallery/undirected/fdpclust.html
  • The Graphviz example shows an undirected graph, so start with graph = pydot.Dot(graph_type='graph') instead of graph = pydot.Dot(graph_type='digraph').
  • Also, you need to render using Graphviz fdp, not dot, to easily get edges between clusters: graph.write_png('undirectedGraphCluster_fdp.png', prog='fdp'). You would have to do some very hacky tricks to accomplish something similar with dot. See this Graphviz FAQ entry, this SO question, Graphviz issue 745 and other issues linked to from there.

Then finally, when all the fixes in pydot and the example code are in place, the end result will look the same as in the Graphviz example:
undirectedGraphCluster_fdp.png

Versions: pydot 1.4.1, python 3.7.3. Mentioning same issue in earlier fork pydot-ng: pydot/pydot-ng#25.

peternowee added a commit to peternowee/pydot that referenced this issue Sep 15, 2020
When instantiating a new `Edge`, if the caller supplies a `Subgraph` or
`Cluster` object as an edge point, store only the name string of that
object, same as we do for `Node` objects as edge points.

According to the DOT Language definition ([1]) an edge point may be a
node or a subgraph. Clusters are just subgraphs whose names begin with
`cluster`, so those are also allowed.

Although not documented, the `Edge` constructor already accepts
subgraphs and clusters as edge points as long as they are in the form
of name strings. These strings are stored in `Edge.obj_dict['points']`
and get included in the writing of a DOT string (`Edge.to_string()`),
just as name strings of nodes would.

However, until now, this is different when an edge point is supplied as
an object. For a `Node`, the name string gets looked up and stored in
`Edge.obj_dict['points']`, but `Subgraph` and `Cluster` objects are
stored there directly as objects.

This leads to problems when `Edge.to_string()` tries to concatenate the
different parts of the edge, with errors like:

    TypeError: unsupported operand type(s) for +: 'Subgraph' and 'str'

    TypeError: sequence item 2: expected str instance, Cluster found

I did not find any explanation for the difference, nor any code that
depends on the points being stored as objects.

The alternative of storing objects across the line was not chosen,
because it would have much larger implications both for the pydot code
itself as for existing user code. Worth considering for the long term
maybe, but this commit provides a bug fix in the current context.

Fixes pydot#89.

[1]: https://www.graphviz.org/doc/info/lang.html
@peternowee
Copy link
Member

peternowee commented Oct 28, 2020

I just rebased #238 on master, no merge conflicts or changes. With regard to the failing AppVeyor tests, see #213, help there would be appreciated.

For the rest, all tests pass and no comments were received, so I will merge it to master.

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

Successfully merging a pull request may close this issue.

3 participants