As part of a project I am working, I needed to read in graph files in the GXL graph format, including their label data.
As this format is not yet supported in this library, I would like to contribute my parser (which I wrote by modifying the existing loadgraphml function). The issue is that, as far as I can tell, there is no existing concept on how to implement graph labels in this library, or even in Graphs.jl.
There is an implementation of a labeled graph in GNNGraphs.jl (the GNNGraph), but I don't think this is the correct implementation to use in this library for two reasons:
- As I understand the code they rely on edge indices for the edge labels, which I believe is not officially supported by Graphs.jl (see this issue)
- It introduces an external dependency that is not really connected to this project
Instead, I have used the following struct to implement labeled graphs, the usage of Dicts inspired by the DataStore in GNNGraphs.jl:
GraphLabels = Dict{String, Any}
struct LabeledGraph
g::AbstractGraph
nlabels::Vector{GraphLabels}
elabels::Matrix{Union{GraphLabels, Missing}}
endI have used a matrix for the edgelabels to avoid having to index the edges. While it would clearly be more storage efficient to use a vector, I believe having to maintain an edge index list would be even more resource intensive than simply using a matrix. Additionally, one can easily extend this to use sparse matrices instead if the graph in question is sparse and large, and it neatly supports both directed and undirected graphs.
Since there is no precedent for this kind of structure, I propose that my parser for this library be split into two versions. One, which simply returns the underlying graph object, and thus conforms to the existing return structure. The other returns a LabeledGraph object as described above. In my opinion this is the best compromise between not parsing the extra label data at all and introducing a new graph object for everyone immediately.
The purpose of this issue is then to ask for opinions on this proposed implementation before I clean up my code for a pull request. Is this acceptable, or are there things I should do differently?
As part of a project I am working, I needed to read in graph files in the GXL graph format, including their label data.
As this format is not yet supported in this library, I would like to contribute my parser (which I wrote by modifying the existing
loadgraphmlfunction). The issue is that, as far as I can tell, there is no existing concept on how to implement graph labels in this library, or even in Graphs.jl.There is an implementation of a labeled graph in GNNGraphs.jl (the
GNNGraph), but I don't think this is the correct implementation to use in this library for two reasons:Instead, I have used the following struct to implement labeled graphs, the usage of Dicts inspired by the
DataStorein GNNGraphs.jl:I have used a matrix for the edgelabels to avoid having to index the edges. While it would clearly be more storage efficient to use a vector, I believe having to maintain an edge index list would be even more resource intensive than simply using a matrix. Additionally, one can easily extend this to use sparse matrices instead if the graph in question is sparse and large, and it neatly supports both directed and undirected graphs.
Since there is no precedent for this kind of structure, I propose that my parser for this library be split into two versions. One, which simply returns the underlying graph object, and thus conforms to the existing return structure. The other returns a LabeledGraph object as described above. In my opinion this is the best compromise between not parsing the extra label data at all and introducing a new graph object for everyone immediately.
The purpose of this issue is then to ask for opinions on this proposed implementation before I clean up my code for a pull request. Is this acceptable, or are there things I should do differently?