KeRNeLith / QuikGraph

Generic Graph Data Structures and Algorithms for .NET
https://kernelith.github.io/QuikGraph/
Microsoft Public License
453 stars 65 forks source link

[BUG] SerializeGraph does not include TaggedEdge data #31

Open Celsiusss opened 3 years ago

Celsiusss commented 3 years ago

Describe the bug Serializing a graph to GraphML, discards TaggedEdge data.

To Reproduce Steps to reproduce the behavior:

  1. Make a graph (I tested with BidirectionalGraph)
  2. Use TaggedEdge
  3. Use graph.SerializeToGraphML
  4. See that the graphml does not have the edge data

Expected behavior Edge data to be included in the graphml as specified here: http://graphml.graphdrawing.org/primer/graphml-primer.html#AttributesDefinition

Additional context Example code to reproduce this problem

Example code: ``` using System; using System.Xml; using QuikGraph; using QuikGraph.Serialization; using System.Xml.Serialization; using System.IO; namespace Test { public class Test { public static void Main() { var graph = new BidirectionalGraph>(); Node node1 = new Node(1, 2); Node node2 = new Node(1, 3); graph.AddVertex(node1); graph.AddVertex(node2); graph.AddEdge(new TaggedEdge(node1, node2, new EdgeData(1))); Save(graph); } private static void Save(BidirectionalGraph> graph) { XmlWriterSettings xmlWriterSettings = new XmlWriterSettings(); xmlWriterSettings.NewLineOnAttributes = true; xmlWriterSettings.Indent = true; var sw = new StringWriter(); XmlWriter writer = XmlWriter.Create(sw, xmlWriterSettings); graph.SerializeToGraphML, BidirectionalGraph>>(writer); writer.Close(); Console.Write(sw.ToString()); } } public class EdgeData { [XmlAttribute("weight")] public float weight { get; set; } public EdgeData(float weight) { this.weight = weight; } } public class Node { [XmlAttribute("x")] public float x { get; set; } [XmlAttribute("y")] public float y { get; set; } public Node(float x, float y) { this.x = x; this.y = y; } } } ```

This code will print out:

<?xml version="1.0" encoding="utf-16"?>
<graphml xmlns="http://graphml.graphdrawing.org/xmlns">
  <key
    id="x"
    for="node"
    attr.name="x"
    attr.type="float" />
  <key
    id="y"
    for="node"
    attr.name="y"
    attr.type="float" />
  <graph
    id="G"
    edgedefault="directed"
    parse.nodes="2"
    parse.edges="1"
    parse.order="nodesfirst"
    parse.nodeids="free"
    parse.edgeids="free">
    <node
      id="0">
      <data
        key="x">1</data>
      <data
        key="y">2</data>
    </node>
    <node
      id="1">
      <data
        key="x">1</data>
      <data
        key="y">3</data>
    </node>
    <edge
      id="0"
      source="0"
      target="1" />
  </graph>
</graphml>

See that this holds vertex data, but no edge data.

KeRNeLith commented 3 years ago

Hello @ende124, Thank you for reporting the bug, I will try to have a look at it as soon as possible. By any chance, did you have some time to investigate a bit? My feeling about it is kindly that serialization is based on Edge class which can explain that TaggedEdge are not taken into account. BTW I need to check it in more details to be sure.

KeRNeLith commented 3 years ago

So I dug a bit in the code related to the GraphML serializer and the problem you're facing is because the serializer indeed not takes into account properties of unsupported types (types specified by the GraphML documentation) and also is limited to properties directly declared in the Edge type you are using. This means that

// This will work
public class CustomEdge : Edge<Node>
{
    [XmlAttribute("weight")]
    public float weight { get; set; }
}

// This will NOT work
public class Data
{
    [XmlAttribute("weight")]
    public float weight { get; set; }
}

public class CustomEdge : TaggedEdge<Node, Data>
{
}

Of course it would be better to have both working. I will try to add support of this. Note that if you have some start of work on the subject feel free to suggest it.

Celsiusss commented 3 years ago

@KeRNeLith That makes sense, I'm glad there is a workaround, it did not cross my mind that one can simply extend Edge. TaggedEdge is preferable, but I have not taken the time to dig into this myself yet. Thank you for looking into this

KeRNeLith commented 3 years ago

@ende124 If the method with a custom Edge fit your needs at least temporarily I think you can go this way. BTW I keep in mind to add support of those kind of Edge (taggued ones) for future releases as it also makes sense.

But I can't promise since it requires quite some rework on the GraphML serializer. But I'm optimistic about feasability ^^

Celsiusss commented 3 years ago

@KeRNeLith yeah that should not be too much of a problem for me. Appreciate your help and effort on this project :)