Subgraphs and contexts are conflated

[james-d-mitchell]

As far as I understand, the notions of subgraphs is one arising in graphviz/dot itself, and contexts are something that we are defining in this GAP package. As such it is a bit surprising that we can use GraphvizSubgraphs to retrieve contexts as in the following from subgraph.tst:

gap> g := GraphvizGraph();;
gap> GraphvizAddContext(g, ["a"]);;
gap> GraphvizSubgraphs(g)[["a"]];
<graphviz context [ "a" ] with 0 nodes and 0 edges>

(note that in my current version of graphviz I removed the function GraphvizGetSubgraph since it duplicates the functionality above).

[james-d-mitchell]

Another example, in GraphvizAddContext there's the following code:

  subgraphs := GraphvizSubgraphs(graph);
  if IsBound(subgraphs[name]) then
    # TODO why are we talking about subgraphs in the error?
    ErrorFormatted("the 1st argument (a graphviz (di)graph) already has ",
                   " a subgraph with name \"{}\"", name);
  fi;

The error message is misleading since it refers to subgraphs, when we were trying to add a context, and the context is being added to GraphvizSubgraphs.

Maybe I misunderstood the difference between contexts and subgraphs, but if not, then it looks like we really need to have a GraphvizContexts function and underlying data structure.

[mpan322]

I agree this is a bit odd.
This behaviour exists as contexts where are essentially identical to subgraphs but with a different 'rendering' behaviour.
The error message is because at the moment subgraphs and contexts share a name space, once again due to the above.

Below is a draft of a rough specification new behaviour. Let me know if you have any concerns or thoughts:

1. The parent of a subgraph will remain whatever the object it was added to (so in principle this could be a context).
2. Subgraphs and contexts will no longer share a namespace.
3. GraphvizSubgraphs will now only return subgraphs of the graph and a new GraphvizContexts will do the same for it's contexts.
4. The subgraphs of a graph will be it's direct children (i.e. not children of it's contexts).

[james-d-mitchell]

Thanks @mpan322, what you propose sounds good (if you mean by same namespace that they are stored in the same component of the object (i.e. Subgraphs or whatever it's called)).

[james-d-mitchell]

If possible can you please make your changes on top of #21? This will make integrating it more straightforward

[james-d-mitchell]

Fixed by #19