Merge IsGraphvizGraph, IsGraphvizDigraph and IsGraphvizContext into a single type

TODO.md

@@ -1,4 +1,36 @@
- - save & load (potentially with 6-strings)
+# Status log
+
+## 2026-10-02
+
+During GAPDays 2026 the graphviz working group made the following contributions
+
+- There have been various improvements to the package infrastructure,
+  improvements to CI, Makefile, PackageInfo
+- The docs have been improved, there are now more examples, the readme now
+  renders all its pictures correctly
+- The splash function has been improved, we fixed an error relating to
+  output directory creation, better error messages
+- Started improving the internals and aligning it more closely with user
+  expectations
+
+Need to do:
+
+- Make the behavior of function match user expectations more
+  - e.g. GraphvizNodes returning all nodes belonging to subgraphs too
+  - Setting attrs applying to the correct contexts
+- Overhaul the internals to simplify functionality
+  (there are parts of code that assume the hierarchy is not a tree when it is)
+- Add better attribute handling
+- Add missing core functionality
+  - Vertex and edge coloring
+  - Better html support
+- Add html handling, use this to improve quotation
+- Big documentation pass
+- Big test pass
+
+# Old todos
+
+- save & load (potentially with 6-strings)
  - condense code in displaying defaults using the new graphviz pkg model
     - ex. all the symmetric vs not symmetric methods can be made a single method which chooses automatically relatively easily.
  - Allow photos to be used as nodes & composition of graphviz output photos (later)

gap/dot.gd

@@ -109,14 +109,16 @@
 #! digraph finite_state_machine {
 #!     rankdir=LR size="8,5"
 #! // terminals context
+#!  {
 #!     node [shape=doublecircle]
 #!     LR_0
 #!     LR_3
 #!     LR_4
 #!     LR_8
 #!     rankdir=LR size="8,5"
-#!
+#!  }
 #! // nodes context
+#!  {
 #!     node [shape=circle]
 #!     LR_2
 #!     LR_0 -> LR_2 [label="SS(B)"]
@@ -138,6 +140,7 @@
 #!     LR_8 -> LR_6 [label="S(b)"]
 #!     LR_8 -> LR_5 [label="S(a)"]
 #!     rankdir=LR size="8,5"
+#!  }
 #!
 #! }
 #! gap> Splash(f);
@@ -240,6 +243,8 @@ DeclareOperation("GraphvizGraph", [IsObject]);
 DeclareOperation("GraphvizGraph", []);
 #! @EndGroup
 
+# TODO: deprecate
+
 #! @BeginGroup
 #! @GroupTitle Creating a new &GAPGraphviz; digraphs
 #! @Arguments name

gap/dot.gi

@@ -13,49 +13,21 @@
 #############################################################################
 
 InstallMethod(GraphvizGraph, "for a string", [IsString],
-function(name)
-  return Objectify(GV_GraphType,
-                      rec(
-                        Name      := name,
-                        Subgraphs := rec(),
-                        Contexts  := rec(),
-                        Nodes     := rec(),
-                        Edges     := [],
-                        Attrs     := rec(graph := rec(),
-                                         edge := rec(),
-                                         node := rec()),
-                        Parent    := fail,
-                        Idx       := 1,
-                        Counter   := 1));
-end);
+name -> GV_GraphOrDigraph(name, IsGraphvizGraph));
 
 InstallMethod(GraphvizGraph, "for an object", [IsObject],
 obj -> GraphvizGraph(String(obj)));
 
 InstallMethod(GraphvizGraph, "for no args", [], {} -> GraphvizGraph(""));
 
 InstallMethod(GraphvizDigraph, "for a string", [IsString],
-function(name)
-  return Objectify(GV_DigraphType,
-                      rec(
-                        Name      := name,
-                        Subgraphs := rec(),
-                        Contexts  := rec(),
-                        Nodes     := rec(),
-                        Edges     := [],
-                        Attrs     := rec(graph := rec(),
-                                         edge := rec(),
-                                         node := rec()),
-                        Parent    := fail,
-                        Idx       := 1,
-                        Counter   := 1));
-end);
-
-InstallMethod(GraphvizDigraph, "for no args", [], {} -> GraphvizDigraph(""));
+name -> GV_GraphOrDigraph(name, IsGraphvizDigraph));
 
 InstallMethod(GraphvizDigraph, "for an object", [IsObject],
 obj -> GraphvizDigraph(String(obj)));
 
+InstallMethod(GraphvizDigraph, "for no args", [], {} -> GraphvizDigraph(""));
+
 #############################################################################
 # ViewString
 #############################################################################
@@ -81,10 +53,10 @@ function(g)
     return false;
   end);
 
-  if IsGraphvizDigraph(g) then
-    kind := "digraph";
-  elif IsGraphvizContext(g) then
+  if IsGraphvizContext(g) then
     kind := "context";
+  elif IsGraphvizDigraph(g) then
+    kind := "digraph";
   else
     kind := "graph";
   fi;
@@ -440,26 +412,16 @@ InstallMethod(GraphvizAddSubgraph,
 "for a graphviz (di)graph or context and string",
 [IsGraphvizGraphDigraphOrContext, IsString],
 function(gv, name)
-  local subgraphs, root, subgraph;
+  local subgraphs, subgraph;
 
   subgraphs := GraphvizSubgraphs(gv);
   if IsBound(subgraphs.(name)) then
     ErrorFormatted("the 1st argument (a graphviz (di)graph/context) ",
                    "already has a subgraph with name \"{}\"", name);
   fi;
 
-  if IsGraphvizContext(gv) then
-    root := GV_EnclosingNonContext(gv);
-  else
-    root := gv;
-  fi;
-
-  if IsGraphvizDigraph(root) then
-    subgraph := GV_Digraph(root, name);
-  elif IsGraphvizGraph(root) then
-    subgraph := GV_Graph(root, name);
-  fi;
-
+  subgraph := GV_Subgraph(gv, name);
+  # TODO(reiniscirpons): Why not just do this in the internal func?
   subgraphs.(name) := subgraph;
   return subgraph;
 end);
@@ -503,6 +465,8 @@ InstallMethod(GraphvizAddContext,
 [IsGraphvizGraphDigraphOrContext, IsObject],
 {g, o} -> GraphvizAddContext(g, String(o)));
 
+# Removal
+
 InstallMethod(GraphvizRemoveNode,
 "for a graphviz (di)graph or context and node",
 [IsGraphvizGraphDigraphOrContext, IsGraphvizNode],

gap/gv.gd

@@ -27,8 +27,10 @@ DeclareOperation("GV_StringifyGraph",
 DeclareOperation("GV_Node", [IsGraphvizGraphDigraphOrContext, IsString]);
 DeclareOperation("GV_Edge",
 [IsGraphvizGraphDigraphOrContext, IsGraphvizNode, IsGraphvizNode]);
-DeclareOperation("GV_Graph", [IsGraphvizGraphDigraphOrContext, IsString]);
-DeclareOperation("GV_Digraph", [IsGraphvizDigraph, IsString]);
+DeclareOperation("GV_GraphOrDigraph", [IsString, IsFunction]);
+DeclareAttribute("GV_Filter", IsGraphvizGraph);
+DeclareAttribute("GV_Filter", IsGraphvizDigraph);
+DeclareOperation("GV_Subgraph", [IsGraphvizGraphDigraphOrContext, IsString]);
 DeclareOperation("GV_Context", [IsGraphvizGraphDigraphOrContext, IsString]);
 
 DeclareOperation("GV_HasNode", [IsGraphvizGraphDigraphOrContext, IsObject]);
@@ -53,22 +55,6 @@ DeclareGlobalFunction("GV_IsValidColor");
 DeclareGlobalFunction("GV_ErrorIfNotNodeColoring");
 DeclareGlobalFunction("GV_ErrorIfNotValidLabel");
 
-# TODO move to dot? and make public?
-BindGlobal("GV_ObjectFamily",
-           NewFamily("GV_ObjectFamily", IsGraphvizObject));
-
-# TODO move to dot? and make public?
-BindGlobal("GV_DigraphType", NewType(GV_ObjectFamily,
-                                    IsGraphvizDigraph and
-                                    IsComponentObjectRep and
-                                    IsAttributeStoringRep));
-
-# TODO move to dot? and make public?
-BindGlobal("GV_GraphType", NewType(GV_ObjectFamily,
-                                    IsGraphvizGraph and
-                                    IsComponentObjectRep and
-                                    IsAttributeStoringRep));
-
 BindGlobal("GV_ValidColorNames",
   ["aliceblue", "antiquewhite", "antiquewhite1", "antiquewhite2",
   "antiquewhite3", "antiquewhite4", "aquamarine", "aquamarine1", "aquamarine2",

gap/gv.gi

@@ -14,20 +14,24 @@
 # Family + type
 ###############################################################################
 
+BindGlobal("GV_ObjectFamily",
+           NewFamily("GV_ObjectFamily", IsGraphvizObject));
+
 BindGlobal("GV_NodeType", NewType(GV_ObjectFamily,
-                                    IsGraphvizNode and
-                                    IsComponentObjectRep and
-                                    IsAttributeStoringRep));
+                                  IsGraphvizNode and
+                                  IsComponentObjectRep and
+                                  IsAttributeStoringRep));
 
 BindGlobal("GV_EdgeType", NewType(GV_ObjectFamily,
-                                    IsGraphvizEdge and
-                                    IsComponentObjectRep and
-                                    IsAttributeStoringRep));
+                                  IsGraphvizEdge and
+                                  IsComponentObjectRep and
+                                  IsAttributeStoringRep));
 
-BindGlobal("GV_ContextType", NewType(GV_ObjectFamily,
-                                    IsGraphvizContext and
-                                    IsComponentObjectRep and
-                                    IsAttributeStoringRep));
+BindGlobal("GV_GraphDigraphOrContextType",
+           NewType(GV_ObjectFamily,
+                   IsGraphvizGraphDigraphOrContext and
+                   IsComponentObjectRep and
+                   IsAttributeStoringRep));
 
 ###############################################################################
 # Constructors etc
@@ -69,27 +73,40 @@ end);
 
 # Graph constructors
 
-InstallMethod(GV_Digraph,
-"for a graphviz digraph and a string",
-[IsGraphvizDigraph, IsString],
-function(parent, name)
-  local out;
-
-  out         := GraphvizDigraph(name);
-  out!.Parent := parent;
-  out!.Idx    := GV_GetCounter(parent);
-
-  GV_IncCounter(parent);
-  return out;
+InstallMethod(GV_GraphOrDigraph,
+"for a string and a filter", [IsString, IsFunction],
+function(name, filt)
+  local result;
+  Assert(0, filt = IsGraphvizGraph or filt = IsGraphvizDigraph);
+
+  result := Objectify(
+    GV_GraphDigraphOrContextType,
+    rec(
+      # Note that the following aren't attributes because we require them to be
+      # mutable.
+      Name      := name,
+      Subgraphs := rec(),
+      Contexts  := rec(),
+      Nodes     := rec(),
+      Edges     := [],
+      Attrs     := rec(graph := rec(),
+                       edge := rec(),
+                       node := rec()),
+      Parent    := fail,
+      Idx       := 1,
+      Counter   := 1));
+  SetFilterObj(result, filt);
+  SetGV_Filter(result, filt);
+  return result;
 end);
 
-InstallMethod(GV_Graph,
+InstallMethod(GV_Subgraph,
 "for a graphviz graph and a string",
 [IsGraphvizGraphDigraphOrContext, IsString],
 function(parent, name)
   local out;
 
-  out         := GraphvizGraph(name);
+  out         := GV_GraphOrDigraph(name, GV_Filter(parent));
   out!.Parent := parent;
   out!.Idx    := GV_GetCounter(parent);
 
@@ -98,26 +115,12 @@ function(parent, name)
 end);
 
 InstallMethod(GV_Context,
-"for a string and a positive integer",
+"for a string and a string",
 [IsGraphvizGraphDigraphOrContext, IsString],
 function(parent, name)
   local out;
-
-  out := Objectify(GV_ContextType,
-                      rec(
-                        Name      := name,
-                        Subgraphs := rec(),
-                        Contexts  := rec(),
-                        Nodes     := rec(),
-                        Edges     := [],
-                        Attrs     := rec(graph := rec(),
-                                         edge := rec(),
-                                         node := rec()),
-                        Parent    := parent,
-                        Idx       := GV_GetCounter(parent),
-                        Counter   := 1));
-
-  GV_IncCounter(parent);
+  out := GV_Subgraph(parent, name);
+  SetFilterObj(out, IsGraphvizContext);
   return out;
 end);
 
@@ -279,8 +282,7 @@ end);
 ###############################################################################
 
 # @ Return DOT graph head line.
-InstallMethod(GV_StringifyGraphHead, "for a string",
-[IsGraphvizGraphDigraphOrContext],
+InstallMethod(GV_StringifyGraphHead, "for a string", [IsGraphvizGraph],
 graph -> StringFormatted("graph {} {{\n", GraphvizName(graph)));
 
 # @ Return DOT digraph head line.
@@ -448,7 +450,7 @@ function(graph, is_subgraph)
     elif IsGraphvizNode(obj) then
       Append(result, GV_StringifyNode(obj));
     elif IsGraphvizEdge(obj) then
-      if IsGraphvizDigraph(GV_GetRoot(graph)) then
+      if (IsGraphvizDigraph(GV_GetRoot(graph))) then
         Append(result, GV_StringifyEdge(obj, "->"));
       else
         Append(result, GV_StringifyEdge(obj, "--"));