Open tcoopman opened 1 year ago
@tcoopman Thanks for contributing! There are really two options - if we want to consolidate implementations for some graph operations, then we can parameterize them on their directionality and move them to a common implementation module, e.g. something like Graph.Common
or Graph.Base
. The alternative is to do as you have started to do, and simply copy the implementations from the Graph.Directed
module to Graph.Undirected
, which is fine, but as you've pointed out, isn't ideal. Would you be interested in tackling the first approach instead? I suspect that for many operations we can simply add an additional argument that is either :directed
or :undirected
, and then dispatch on that if the behavior differs. For operations where the behavior has little overlap, then defining those functions in Graph.Undirected
makes sense to me.
In either case, I appreciate the contribution, so take whichever approach is preferable for you at this point, and if need be it can always be refactored later.
I will maybe pick it up at some point again, but I currently don't have the time for it.
I like the idea of a common graph module and operations with directed and undirected as arguments for the functions. Although I'm not sure if that should be exposed as the graph type is already directed or undirected.
I'm also not sure if all functions should have an implementation for both types. For example reachable_neighbors should probably be specified differently for undirected graphs, because you always have a path back.
@bitwalker is this what you had in mind?
A common module which delegates function to directed or undirected?
As a proof of concept, I've started to implement some Undirected functions (see #39). I've did it by copying all things over from
Directed
and changing some parts. This is not optimal.If you feel like this is useful, I might add more functions.