WIP: GraphML MetaGraph support #49
Closed
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
as mentioned in #47.
This PR enables support for
MetaGraphsinGraphMLformat.It follows the paradigm of http://graphml.graphdrawing.org/ to define attributes in the graph.
The following features are supported:
Dictof graphsAlthough it's functional, I marked it as WIP because I haven't really thought too much about performance and type-stability and more importantly I think some discussion is needed in advance.
Caveats
MetaGraphsEzXML.StreamReaderfor the reason that there needs to be some memory on the parser in order to remember whose are the properties readGraphIO.GraphMLand not fromGraphs. The reason is that now the developer needs also to mention the desired graph type s/he wishes to parse (in this case::MGFormat).Discussion
I know that currently there is some confusion with the architecture choices of
GraphIO.jl.For example there is the problem with
requireas in #42.I think it's important to find a solution, in order to really take advantage of future contributions.
Regarding graph IO parsing, I see 2 cases:
Up until now the 2. was not supported but it is necessary for e.g. parsing
MetaGraphs.To support 2. I just added one argument to the
loadgraph(which makes it inaccessible just from Graphs.jl)So, we probably also need to add this definition in
Graphs.jl.(offtopic)- Wouldn't it make sense to move all the graph IO code from
Graphs.jltoGraphIO.jl?Personally, I find it unsustainable to put everything in one package.
We can't possibly support all graph types, since this will unnecessary grow the dependencies.
Starting with
MetaGraphs, I find it better to have it in a separate package.For example I could put the code into something like
MetaGraphsIO.jl.And probably we would need to mimic
GraphIO.jland inside the hypotheticalMetaGraphsIOsupport all type formats (GraphML,GML,DOT, etc.).(ofc I am not sure if all type formats support additional attributes like
GraphMLdoes, so probably it will be a subset)Plans
In future work I want to include also nested graphs support for
GraphMLwith a custom nested graph type that is still not public.Also, since
GraphMLattributes are type-secure, this makes it a good fit forMetaGraphsNextwhich I will also try to support in the future.This means that I could end up creating
MyWeirdNestedGraphIO.jlandMetaGraphsNextIO.jlBefore I move to it, I would really appreciate some discussion and advices.
That said, I think we need to build the infrastructure to welcome future work in this direction.
Sorry for the long text :)