Add more convenience constructors
The type GNNGraph from GraphNeuralNetworks.jl is a Graphs.AbstractGraphs and implements the corresponding interface, therefore it would be nice to make the following code work by providing an appropriate constructor:
julia> using GraphNeuralNetworks, Graphs
julia> g = GraphNeuralNetworks.rand_graph(10, 20)
GNNGraph:
num_nodes = 10
num_edges = 20
julia> g isa Graphs.AbstractGraph
true
julia> Graphs.SimpleGraph(g)
ERROR: MethodError: no method matching SimpleGraph(::GNNGraph{Tuple{Vector{Int64}, Vector{Int64}, Nothing}})
Closest candidates are:
SimpleGraph(::Any, ::Array{Vector{T}, 1}) where T at ~/.julia/packages/Graphs/pxGmw/src/SimpleGraphs/simplegraph.jl:18
SimpleGraph() at ~/.julia/packages/Graphs/pxGmw/src/SimpleGraphs/simplegraph.jl:50
SimpleGraph(::SimpleGraph) at ~/.julia/packages/Graphs/pxGmw/src/SimpleGraphs/simplegraph.jl:115
...
Moreover, we could also have the following constructor:
julia> src, dest = edge_index(g)
([1, 1, 1, 1, 2, 2, 2, 5, 5, 5, 2, 3, 7, 10, 5, 6, 7, 6, 7, 9], [2, 3, 7, 10, 5, 6, 7, 6, 7, 9, 1, 1, 1, 1, 2, 2, 2, 5, 5, 5])
julia> SimpleGraph(src, dest)
ERROR: MethodError: no method matching SimpleGraph(::Vector{Int64}, ::Vector{Int64})
Closest candidates are:
SimpleGraph(::Any, ::Array{Vector{T}, 1}) where T at ~/.julia/packages/Graphs/pxGmw/src/SimpleGraphs/simplegraph.jl:18
which is more concise and discoverable than
julia> SimpleGraph(Edge.(src,dest))
{10, 10} undirected simple Int64 graph
Is it a bug though? Not rather a new feature?
I fully agree with your first point, there is also an older issue for that:: https://github.com/JuliaGraphs/Graphs.jl/issues/98
About the second part, I feel like we have too many constructors as it might start to get confusing, so I am in favor of also creating factory functions with different names, such as SimpleGraphFromIteratator that we had to create because of some type issue.
But we probably should also support tuples more. Then one could also write SimpleGraph(zip(src, dst)) in your second example.
Is it a bug though? Not rather a new feature?
sorry, I accidentally clicked on the "bug" button when opening an issue and now I cannot remove the label.
About the second part, I feel like we have too many constructors
SimpleGraph(src::AbstractVector{<:Integer}, dst::AbstractVector{<:Integer})
is pretty natural and semantically unambiguous. It is something that you can find out by yourself without digging in the documentation.
SimpleGraph((src, dest)::Tuple{AbstractVector{<:Integer}, AbstractVector{<:Integer}})
is another option
PR #301 tackles the first part of your request, can you take a look @CarloLucibello? PR #252 might address the second part but I'm not gonna merge that one yet