Add VertexDataGraph and EdgeDataGraph as concrete subtypes of AbstractVertexOrEdgeDataGraph#121
Add VertexDataGraph and EdgeDataGraph as concrete subtypes of AbstractVertexOrEdgeDataGraph#121jack-dunham wants to merge 23 commits into
VertexDataGraph and EdgeDataGraph as concrete subtypes of AbstractVertexOrEdgeDataGraph#121Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #121 +/- ##
==========================================
+ Coverage 59.89% 66.35% +6.46%
==========================================
Files 10 13 +3
Lines 566 853 +287
==========================================
+ Hits 339 566 +227
- Misses 227 287 +60
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…hrow(); end; ...`
…eturn similar types.
| function GraphsExtensions.forest_cover_edge_sequence(graph::AbstractDataGraph; kwargs...) | ||
| dummy_graph = add_edges!(NamedGraph(vertices(graph)), edges(graph)) | ||
| return GraphsExtensions.forest_cover_edge_sequence(dummy_graph; kwargs...) | ||
| end |
There was a problem hiding this comment.
Is this needed? Shouldn't the generic definition "just work"?
There was a problem hiding this comment.
No because it's going to try and remove edges and that might not be defined. It is easier to just construct a dummy named graph as all this function does is return a list of edges.
Really it should return a list of edges of the same type as the input graph, though so maybe this needs to be rethought.
There was a problem hiding this comment.
I see. Probably that means that function should be rewritten to "just work" even for AbstractDataGraph, i.e. at worst it could first call add_edges!(NamedGraph(vertices(graph)), edges(graph)) and then run the rest of the function.
There was a problem hiding this comment.
To be honest I accidentally introduced an infinite method dispatch loop w.r.t this function in NamedGraphs so I need to make another PR anyway...
This point of this abstract interface is to allow the partial implementation of the
Dictionaries.jlinterface, on these graph types.VertexDataGraphandEdgeDataGraphas concrete subtypes ofAbstractVertexOrEdgeDataGraphAbstractVertexDataGraphandAbstractEdgeDataGraphinterfaces.insert_vertex_datathat should be overloaded if a givenAbstractVertexDataGraphgraphisinsertable. See below as to why the analogous interface function doesn't exist forAbstractEdgeDataGraph.A subtype of
AbstractVertexOrEdgeDataGraphimplements a stricter indexing interface, inline with the one used byDictionaries.jl, that issetindex!strictly sets existing indices (doesn't add a vertex/edge that isn't already in the graph). One should useinsert!to strictly add a new vertex/edge with data, andset!to perform anupsert(previous behavior ofsetindex!.Eventually, this interface will be applied to
AbstractDataGraph, but to avoid breaking changes this is deferred until a later date.Subtleties
Difference in
setindex!for vertex and edge dataThe
setindex!function on anAbstractEdgeDataGraphshould add an edge if doesn't exist already. This is unlikesetindex!for anAbstractVertexDataGraph, which errors if there vertex is not present. This is in analogy to SparseArrays:For an edge data graph,
setindex!will only error if the vertices associated to an edge aren't present (irrespective of the existence of an edge).Difference in
insert!anddelete!for vertex and edge dataFor a vertex data graph, the function
insert!inserts a vertex with data, erroring if it already exits. For an edge data graph,insert!also inserts the required vertices and then adds the edge with data, erroring if both vertices are already present. That is,insert!errors wheresetindex!would not error. The functiondelete!deletes the edge and data only, leaving the data untouched.Interface differences
For a settable and insertable vertex data graph, one must define:
An edge data graph only needs:
As
insert!is defined in terms ofadd_vertex!andset!(which can create edges).