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

How to add ports for edges (GraphViz) #92

Closed
clemens-tolboom opened this issue Dec 29, 2013 · 13 comments
Closed

How to add ports for edges (GraphViz) #92

clemens-tolboom opened this issue Dec 29, 2013 · 13 comments
Assignees
Labels

Comments

@clemens-tolboom
Copy link
Collaborator

In #36 a commented on the record structure which allows for 'deep' edge snap points.

Using tailport and headport for an edge we can do this.
tail-port-label-head-2

digraph {

  node [
    shape=record
  ];

  source [
    label="<f0> left |<middle> middle |<f2> right"
    shape=record
  ]

  target [
    label="<f0> left |<f1> middle |<right> right"
    shape=record
  ]

  source -> target [
    tailport = middle
    headport = right
    taillabel = source
    label = middle
    headlabel = target
  ]

}

Having a vertex label like

node [shape=record];
struct1 [label="<f0> left | <f1> middle | <f2> right"];
another

we can connect an edge to f1 like this

a -> struct1:f1

Using portpos like

a -> struct1:f1:nw;

is even worse.

AFAIK we don't have support for this.

Defining this as a Layout is not an option either?

XREF

@clue
Copy link
Member

clue commented Dec 29, 2013

Thanks for bringing this to my attention! Adding some kind of support for GraphViz' port definitions certainly sounds tempting.

Currently, just merely adding vertex ports requires a deeper understanding of GraphViz record/Mrecord structures or HTML labels, as in your above example. Personally, I think this is not quite ideal, but it's okay considering this is a general purpose graph library and not just a GraphViz dot abstraction. Should we choose to stick to that, I think it makes sense to add some kind of special layout attributes like this:

$a = $graph->getVertex('a');
$struct1 = $graph->getVertex('struct1');
$edge = $a->createEdgeTo($struct1);

$edge->getLayout()->setAttribute('port.target', 'f1');

We would then have to update our GraphViz class to filter out these edge layout attributes and rewrite the dot output accordingly.

I've updated the PR title to reflect the feature request.

@clemens-tolboom
Copy link
Collaborator Author

I agree with we have a graph lib and not a graphviz lib. And settings attributes for graphviz is then the way to do this.

Some remarks:

  • Is port.target specific enough? Is the target vertex always the second vertex? They cannot be swapped?
  • I prefer using an _ to distinguish from real graphviz attributes. Unfortunately graphviz has _background. So let's take . like .port.target

I'll write a test for this :)

Is there a way to attach this issue to a PR?

@ghost ghost assigned clemens-tolboom Dec 30, 2013
@clue
Copy link
Member

clue commented Dec 30, 2013

Is port.target specific enough?

Probably not :> This might seem okay for directed edges, whereas there's no distinguished "target" for an undirected one. Any suggestions?

[…] to distinguish from real graphviz attributes.

Indeed! Prefixing this might be a good idea. Considering this feature is limited to GraphViz, perhaps prefix this with graphviz.?

Is there a way to attach this issue to a PR?

Just open a new PR and include "fixes #xx" in the description.

@clemens-tolboom
Copy link
Collaborator Author

Is port.target specific enough?

Probably not :> This might seem okay for directed edges, whereas there's no distinguished "target" for an undirected one. Any suggestions?

Reading http://www.graphviz.org/doc/info/attrs.html#h:undir_note we can relax on not working as graphviz states

To avoid possible confusion when such attributes are required, the user is encouraged to use a directed graph.

That means we should not care :-/ ... OTOH we prob could help our users with taking care for upgrading a undirected to a directed graph. But that's another issue.

Prefixing this might a good idea. Considering this feature is limited to GraphViz, perhaps prefix this with graphviz.?

Great suggestion graphviz.

Just open a new PR and include "fixes #xx" in the description.

Always nice to learn new stuff :)

@clue
Copy link
Member

clue commented Jan 5, 2014

Is port.target specific enough?

Probably not :> This might seem okay for directed edges, whereas there's no distinguished "target" for an undirected one. Any suggestions?

Reading http://www.graphviz.org/doc/info/attrs.html#h:undir_note we can relax on not working as graphviz states

GraphViz documentation certainly makes a valid point. Now even irrespective of what GraphViz does, what should our API look like? Using the words "target" and "source" will ultimately be just as ambiguous in our context, as there's no such concept for an undirected edge.

Unless we can find a better wording I'm kind of okay with that, provided we add some documentation.

upgrading a undirected to a directed graph

I don't think that's an option here. Adding two directed edges instead of a single undirected one certainly leads to a whole new level of issues. Besides, this doesn't actually fix our naming issue, as we'd still have to apply the port definition on the original undirected edge.

@clemens-tolboom
Copy link
Collaborator Author

When we regard vertex - edge - vertex we can easily express vertex - (target:f1) - edge - (target:f6) - vertex

That is the relation vertex - edge has a layout / attribute. That is probably a no-go. Trouble is in example an ERD too has arrow type on both sides.

We are save if we only proces layout values on DirectedEdge only.

These would workout ok I guess:

graphviz.from.target = f1
graphviz.from.arrow = diamond
graphviz.to.target = f4
graphviz.to.arrow = vee

Let's wait for this to solve after moving Graphviz out of this repo?

@clemens-tolboom
Copy link
Collaborator Author

@clue
Copy link
Member

clue commented Jan 7, 2014

arrow type on both sides

Thanks for the links. Arrows use the normal layout attribute settings and can be changed as usual:

$edge->getLayout()->setAttribute('arrowtail', 'diamond')->setAttribute('arrowhead', 'vee');

Also, regarding port definitions: After some further inspection it turns out the following definition

source:sourceport -> target:targetport

is equivalent to

source -> target [tailport=sourceport, headport=targetport]

As such, your initial example can be expressed like this:

// a -> struct1:f1
$edge = $va->createEdgeTo($vstruct1);
$edge->getLayout()->setAttribute('headport', 'f1');

Would you care to confirm this? If so, I'd like to keep this as-is and consider this bug done. Perhaps we should add some examples (separate repo? #97) to show its usage?

@clemens-tolboom
Copy link
Collaborator Author

Nice:)

I've updated the issue summary.

@clemens-tolboom
Copy link
Collaborator Author

When we have new repo let's add code examples (read tests) generating all our puzzles.

My current practice is to generate artifacts in the tests directory. People interested in those output just have to run tests.

Created a new milestone to move issue 'out of sight'

@clue
Copy link
Member

clue commented Jan 9, 2014

Yep, once #97 is done, we should probably add some layout tests.

Other than that, I assume this ticket can be closed?

@clemens-tolboom
Copy link
Collaborator Author

Nope ... we have to build examples and this is one of them.

@clue
Copy link
Member

clue commented Jan 30, 2014

we have to build examples

I agree. Considering we're about to split off graphviz to a separate project (#97), I've linked this ticket to a dedicated documentation ticket there (graphp/graphviz#2).

So closing this in the core graph library.

@clue clue closed this as completed Jan 30, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants