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

Store Subgraph and Cluster edge points by name #238

Merged
merged 1 commit into from
Oct 28, 2020

Conversation

peternowee
Copy link
Member

@peternowee peternowee commented Sep 14, 2020

Commit message:
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 #89.

Further remarks:
@prmtl @ankostis or others: Reviews/comments still very much welcome of course!

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 peternowee merged commit 03533f3 into pydot:master Oct 28, 2020
@peternowee
Copy link
Member Author

With regard to the failing AppVeyor tests, see #213, help there would be appreciated.

@peternowee peternowee deleted the fix/cluster-subgraph-edge branch November 16, 2020 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cluster to cluster edge fails
1 participant