New similar_graph interface for AbstractDataGraph; misc bug fixes.

[jack-dunham]

This PR includes some bug fixes and a new AbstractDataGraph specific method for the function similar_graph from the NamedGraphs.GraphsExtensions library.

• Remove the similar_graph methods that take underlying_graph as an input, and reconsider parts of the code where this is used.
• Removed set_underlying_graph function
• GraphsExtensions.directed_graph now returns a default (say NamedDiGraph in that case) unless specified for concrete types (like DataGraph).

[mtfishman]

@jack-dunham I have to admit I've gotten a bit lost here in terms of how the functionality in this PR will be used. Where is similar_graph on data graphs actually used? It may help to focus on that so we can cut down on the number of methods we are defining here, and decide which methods we even need.

[mtfishman]

To be specific, if we take the analogy with similar for arrays seriously, I think it would leave us with:

similar_graph(g::AbstractDataGraph) # similar(a::AbstractArray)
similar_graph(g::AbstractDataGraph, vertices, edges) # similar(a::AbstractArray, axes)
similar_graph(g::AbstractDataGraph, vertex_data_type::Type, edge_data_type::Type) # similar(a::AbstractArray, T::Type)
similar_graph(g::AbstractDataGraph, vertex_data_type::Type, edge_data_type::Type, vertices, edges) # similar(a::AbstractArray, T::Type, axes)

# Type version
similar_graph(graph_type::Type{<:AbstractDataGraph}, vertices, edges) # similar(array_type::Type{<:AbstractArray}, axes)

Underlying graph feels a little "low level" to expose to this kind of interface (maybe it is needed but it would be nice to think through that). I think one application of that is to make a similar graph that changes directedness, i.e. specifies it should be directed or undirected, but maybe that could be handled in another way (like with a trait input like Directed()/Undirected() or a certain way of specifying the edges, i.e. DirectedEdges([...]) vs. UndirectedEdges([...])).

Also, how does this interface handle data graphs that only have edge or vertex data? I think that would be a pretty big source of ambiguity so is worth considering here. That would be a compelling reason to base things around trait wrappers like VertexType, EdgeType, etc. (like the proposal here: JuliaLang/julia#18161). EDIT: I guess in those cases we'd want to introduce an AbstractVertexDataGraph and AbstractEdgeDataGraph so we could dispatch similar_graph on those and they could have different signatures, so maybe not something to worry too much about right now, but something to keep in mind).

[jack-dunham]

I agree with everything you are saying. For some reason I had it in my head that you could do:

similar(Matrix, Float64, (2, 2))

but this is not the case. One should do

similar(Matrix{Float64}, (2,2))

Instead we can have (in future):

similar_graph_type(::Type{<:AbstractDataGraph}, VD::Type, ED::Type, G::Type)

which returns a Type , in analogy to the similar_type function implemented in some packages. Therefore all we need is:

similar_graph(::Type{<:AbstractDataGraph}, vertices, edges)

the rest of the type domain methods can be removed (with the prospect of later adding similar_graph_type if needed/wanted.

As for underlying graphs we can just have the methods:

similar_graph(::AbstractDataGraph, underlying_graph::AbstractGraph)
similar_graph(::Type{<:AbstractDataGraph}, underlying_graph::AbstractGraph)

and avoid the combinatorics explosion of combing this with all the other possible arguments. I do appreciate your point regarding underlying graph being too "low level", but the existence of such a graph is explicitly part of the AbstractDataGraphs interface at the moment. I think it would be a good idea to reevaluate that in favor of a function like dataless_graph (or something) that returns the underlying graph, or constructs a dataless graph on the fly (dependent on the AbstactDataGraph), in a similar way to what we did with the vertex/edge data storage, but for now I think the above two methods are justified by the existing interface, and are used in the code.

Thanks for the comments.

[jack-dunham]

Also, how does this interface handle data graphs that only have edge or vertex data? I think that would be a pretty big source of ambiguity so is worth considering here. That would be a compelling reason to base things around trait wrappers like VertexType, EdgeType, etc. (like the proposal here: JuliaLang/julia#18161). EDIT: I guess in those cases we'd want to introduce an AbstractVertexDataGraph and AbstractEdgeDataGraph so we could dispatch similar_graph on those and they could have different signatures, so maybe not something to worry too much about right now, but something to keep in mind).

Currently, subtypes that do not have e.g. edge data have edge data type Nothing, as the AbstractDataGraph requires both a vertex and edge data type parameter. So one would have to do:

struct GraphWithoutEdgeData{V, VD} <: AbstractDataGraph{V, VD, Nothing} end

and:

similar_graph(graph_without_edge_data, VD, Nothing) 

or for the edge data case:

similar_graph(graph_without_vertex_data, Nothing, ED) 

Of course given graph_with_edge_data::GraphWithoutEdgeData, one could just define methods like:

similar_graph(graph::GraphWithoutEdgeData, VD::Type) = similar_graph(graph, VD, Nothing)

etc. which could be done abstractly using AbstractVertexDataGraph.

[jack-dunham]

To record conversations had on slack, the implemented similar_graph interface is now

similar_graph(g::AbstractDataGraph) # similar(a::AbstractArray)
similar_graph(g::AbstractDataGraph, vertices) # similar(a::AbstractArray, axes)
similar_graph(g::AbstractDataGraph, data_type::Type) # similar(a::AbstractArray, T::Type, axes)
similar_graph(g::AbstractDataGraph, data_type::Type, vertices) # similar(a::AbstractArray, T::Type)
similar_graph(g::AbstractDataGraph, vertex_data_type::Type, edge_data_type::Type) # similar(a::AbstractArray, T::Type)
similar_graph(g::AbstractDataGraph, vertex_data_type::Type, edge_data_type::Type, vertices) # similar(a::AbstractArray, T::Type, axes)

# Type version (defined abstractly in `NamedGraphs`.)
similar_graph(graph_type::Type{<:AbstractDataGraph}, vertices) # similar(array_type::Type{<:AbstractArray}, axes)

i.e. we have no methods taking the underlying graph as an argument. This is in anticipation of removing underlying_graph from the interface of AbstractDataGraph in a future PR.

[mtfishman]

Looks like test failures are caused by recent changes to the shared test workflow, I'll investigate.