[BUG] GraphvizVertexShape.Record causes Label string to be empty on output.

[squishleheimer]

When using ToGraphviz and the FormatVertex handler, when you assign the args.VertexFormat.Shape the value GraphvizVertexShape.Record, when the dot is output, the Label ends up empty despite assigning any string value to the Label, let alone that you would expect to be able to use the record formatting on. i.e. "{ a | b | c }".

e.g. Expected gv output:

...
0 [shape=record, label="{ a | b | c }"];

Actual gv output:

...
0 [shape=record, label=""];

[KeRNeLith]

Hello,

First of all thank you for your report!

Hum this is certainly not enough documented but in order to create record label you need to setup the vertex Shape to GraphvizVertexShape.Record indeed but rather than setting the Label you need to feed the Record property which is a more complex structure that allows to describe complex records. Here is a working example in which I also marked what is still an issue:

graph.ToGraphviz(algorithm =>
{
    algorithm.CommonVertexFormat.Shape = GraphvizVertexShape.Record; // <= If you set this without setting it in the FormatVertex then it will indeed not work (good catch ;-))
    algorithm.FormatVertex += (_, args) =>
    {
        args.VertexFormat.Shape = GraphvizVertexShape.Record;  // <= For now it is important to specify it here also
        // Below some record setup (see below for how it looks like)
        args.VertexFormat.Record = new GraphvizRecord
        {
            Cells = new GraphvizRecordCellCollection(new[]
            {
                new GraphvizRecordCell { Text = "Test Cell1" },
                new GraphvizRecordCell { Port = "Test Port2", Text = "Test Cell2" },
                new GraphvizRecordCell
                {
                    Cells = new GraphvizRecordCellCollection(new[]
                    {
                        new GraphvizRecordCell
                        {
                            Port = "Sub Test Port1", Text = "Sub Test Cell 1"
                        },
                        new GraphvizRecordCell
                        {
                            Port = "Sub Test Port2", Text = "Sub Test Cell 2"
                        }
                    })
                }
            })
        };
    };
});

Here is the the visual aspect of example record:

BTW using the common vertex format to setup stuff must allow to configure record and not have to repeat it like in my sample. This is something that need to be fixed anyway.

[squishleheimer]

Thanks for that. I actually discovered the above after raising the issue. As you say, it may make sense to check the Label value of the vert for null or whitespace before compiling a string from the Record cell collection.

[KeRNeLith]

Rather than checking the Label value, I think it's better to rely on the selected Shape for first the vertex itself, and then the common vertex format. This is because regardless of the Label value, it will always be computed as a record if shape is defined to GraphvizVertexShape.Record. See this for the precise location where it's handled.

[squishleheimer]

Sorry, what I meant was something like this, in the location that you indicated:

if (Shape == GraphvizVertexShape.Record)
{
    properties["label"] = string.IsNullOrWhiteSpace(Label) ? Record : EscapeRecord(Label);
}

[KeRNeLith]

Oh you mean to prefer Label if set, even if using Record shape. I imagine you generating your own label that is record formatted, right?

[squishleheimer]

Yes. I'm assuming that if, for example, the vert label is set directly with a literal "{ a | b | c }", then it would allow it through to the dot engine. So if you did allocate the Record structure then you'd need to ensure that you left label as null or empty if you wanted the original behaviour (previous to suggested fix) to still set the label. Seems like a reasonable quick fix but I will leave it to you to decide.

[squishleheimer]

Ah, I forgot about this bit that will need attention - so not quite as quick-fix as I thought but not too bad.

algorithm.CommonVertexFormat.Shape = GraphvizVertexShape.Record; // <= If you set this without setting it in the FormatVertex then it will indeed not work (good catch ;-))

[KeRNeLith]

I think we can indeed go this way for the Label/Record management as it still allows to use Record structure but also extends to custom generated record, while enforcing the logic to not set both at the same time to avoid conflicts.

[squishleheimer]

Looks like there's a problem publishing artifacts to nuget? https://ci.appveyor.com/project/KeRNeLith/quikgraph/build/job/f7222dyf51ipbuol/messages @KeRNeLith

[KeRNeLith]

That's not publish to NuGet.org but to MyGet for now (snapshot). Note that it seems to be an issue on Microsoft side for symbol packages, because it's not the real package that trigger those errors. On my side if I'm not wrong I set authors as requested, the problem is the procedure to geneate symbol package that voluntarily skips/removes this information. Didn't remember the reason that was given by Microsoft team. Anyway this should not be an issue but it would be nice if they update nuget.exe to not raise those errors while being in their standard process. I may have to re-check if there were changes for this by the way.

[squishleheimer]

OK. So how does 2.3.1 happen then? :shipit:

[KeRNeLith]

As I said I prefer including a fix to properly generate a graphviz representation taking into account the common vertex format setup. The first commit was to apply the priority over Label we discussed ealier. I'm investigating a way to properly do this. Meanwhile if you have any suggestion/working version feel free to propose it ;-)

[KeRNeLith]

Issues discussed on the thread are fixed by fcf545f2cc840bae799128c3e446c91cf8e8a696 and cd1be75972cf20784bd75b95e7da98ff1d1296cc. They are available on releases >= 2.3.1.