From ca1ee83e383ee8ca6f5f0b9cb9637d98baf2119e Mon Sep 17 00:00:00 2001 From: James Mitchell Date: Thu, 1 Oct 2026 15:31:34 +0100 Subject: [PATCH 1/5] Remove \[\] access from nodes --- gap/dot.gd | 2 -- gap/dot.gi | 17 ----------------- tst/node.tst | 18 +++++++++--------- 3 files changed, 9 insertions(+), 28 deletions(-) diff --git a/gap/dot.gd b/gap/dot.gd index 62b2968..58f01fa 100644 --- a/gap/dot.gd +++ b/gap/dot.gd @@ -628,8 +628,6 @@ DeclareGlobalFunction("ErrorIfNotValidColor"); #! @BeginExampleSession #! @EndExampleSession DeclareOperation("\[\]", [IsGraphvizEdge, IsObject]); -#! @Arguments node, attr -DeclareOperation("\[\]", [IsGraphvizNode, IsObject]); #! @EndGroup #! @BeginGroup diff --git a/gap/dot.gi b/gap/dot.gi index c133c69..86527ef 100644 --- a/gap/dot.gi +++ b/gap/dot.gi @@ -159,23 +159,6 @@ x -> x!.Head); InstallMethod(\=, "for graphviz nodes", [IsGraphvizNode, IsGraphvizNode], IsIdenticalObj); -# Accessing node attributes - -# TODO Remove this, and just use GraphvizAttrs(node) directly? -InstallMethod(\[\], "for a graphviz node and a string", -[IsGraphvizNode, IsString], -function(node, key) - if IsBound(GraphvizAttrs(node).(key)) then - return GraphvizAttrs(node).(key); - fi; - return fail; -end); - -# TODO Remove this, and just use GraphvizAttrs(node) directly? -InstallMethod(\[\], "for a graphviz node and an object", -[IsGraphvizNode, IsObject], -{node, key} -> node[String(key)]); - # Setting node attributes # TODO Remove this, and just use GraphvizAttrs(node) directly? diff --git a/tst/node.tst b/tst/node.tst index 49bb8be..8345415 100644 --- a/tst/node.tst +++ b/tst/node.tst @@ -63,16 +63,16 @@ gap> n := GraphvizAddNode(g, 111); gap> AsString(g); "//dot\ngraph {\n\t111\n}\n" -gap> n[1]; -fail -gap> n[1] := 2; -2 -gap> n[1]; +gap> IsBound(GraphvizAttrs(n).1); +false +gap> GraphvizSetAttr(n, 1, 2); + +gap> GraphvizAttrs(n).1; "2" gap> GraphvizRemoveAttr(n, 1); -gap> n[1]; -fail +gap> IsBound(GraphvizAttrs(n).1); +false # Test removing a node with a non-string name gap> g := GraphvizGraph();; @@ -97,8 +97,8 @@ rec( color := "blue", label := "1" ) # Test getting attributes using the [] syntax gap> g := GraphvizGraph();; gap> n := GraphvizAddNode(g, "a");; -gap> n["color"] := "red";; -gap> n["color"]; +gap> GraphvizSetAttr(n, "color", "red");; +gap> GraphvizAttrs(n).color; "red" # Test set label (node) From e89211733b3fc4753df5e5699f24996df33fb590 Mon Sep 17 00:00:00 2001 From: James Mitchell Date: Thu, 1 Oct 2026 15:36:51 +0100 Subject: [PATCH 2/5] Remove \[\]:= assignment for nodes --- gap/dot.gd | 1 - gap/dot.gi | 16 ---------------- tst/node.tst | 6 +++--- 3 files changed, 3 insertions(+), 20 deletions(-) diff --git a/gap/dot.gd b/gap/dot.gd index 58f01fa..217a98c 100644 --- a/gap/dot.gd +++ b/gap/dot.gd @@ -638,7 +638,6 @@ DeclareOperation("\[\]", [IsGraphvizEdge, IsObject]); #! Sets the value associated with the attribute attr. #! @BeginExampleSession #! @EndExampleSession -DeclareOperation("\[\]\:\=", [IsGraphvizNode, IsObject, IsObject]); #! @Arguments edge, attr DeclareOperation("\[\]\:\=", [IsGraphvizEdge, IsObject, IsObject]); #! @EndGroup diff --git a/gap/dot.gi b/gap/dot.gi index 86527ef..994146e 100644 --- a/gap/dot.gi +++ b/gap/dot.gi @@ -159,22 +159,6 @@ x -> x!.Head); InstallMethod(\=, "for graphviz nodes", [IsGraphvizNode, IsGraphvizNode], IsIdenticalObj); -# Setting node attributes - -# TODO Remove this, and just use GraphvizAttrs(node) directly? -InstallMethod(\[\]\:\=, "for a graphviz node and two strings", -[IsGraphvizNode, IsString, IsString], -function(node, key, val) - GraphvizAttrs(node).(key) := val; -end); - -# TODO Remove this, and just use SetGraphvizAttr(node, key, val) directly? -InstallMethod(\[\]\:\=, "for a graphviz node and two strings", -[IsGraphvizNode, IsObject, IsObject], -function(node, key, val) - node[String(key)] := String(val); -end); - # Accessing edge attributes # TODO Remove this, and just use GraphvizAttr(edge) directly? diff --git a/tst/node.tst b/tst/node.tst index 8345415..62f99b5 100644 --- a/tst/node.tst +++ b/tst/node.tst @@ -84,13 +84,13 @@ rec( ) # Test setting attributes using the []:= syntax gap> g := GraphvizGraph();; gap> n := GraphvizAddNode(g, "a");; -gap> n["color"] := "red";; +gap> GraphvizSetAttr(n, "color", "red");; gap> GraphvizAttrs(n); rec( color := "red" ) -gap> n["label"] := 1;; +gap> GraphvizSetAttr(n, "label", 1);; gap> GraphvizAttrs(n); rec( color := "red", label := "1" ) -gap> n["color"] := "blue";; +gap> GraphvizSetAttr(n, "color", "blue");; gap> GraphvizAttrs(n); rec( color := "blue", label := "1" ) From b5bf1115bdcdf184519049f508453c18616dd92f Mon Sep 17 00:00:00 2001 From: James Mitchell Date: Thu, 1 Oct 2026 15:49:26 +0100 Subject: [PATCH 3/5] Add GraphvizGetAttr --- gap/dot.gd | 9 +++++++++ gap/dot.gi | 13 +++++++++++++ tst/edge.tst | 10 +++++----- tst/node.tst | 16 ++++++++-------- 4 files changed, 35 insertions(+), 13 deletions(-) diff --git a/gap/dot.gd b/gap/dot.gd index 217a98c..05984a3 100644 --- a/gap/dot.gd +++ b/gap/dot.gd @@ -557,6 +557,15 @@ DeclareOperation("GraphvizSetAttr", [IsGraphvizObject, IsObject, IsObject]); DeclareOperation("GraphvizSetAttr", [IsGraphvizObject, IsObject]); #! @EndGroup +#! @Arguments gv, obj +#! @Returns the value of an attribute or fail. +#! @Description +#! This operation returns the current value of the attribute obtained from +#! String(obj), or fail if there's none. +#! @BeginExampleSession +#! @EndExampleSession +DeclareOperation("GraphvizGetAttr", [IsGraphvizObject, IsObject]); + #! @Arguments obj, attr #! @Returns a &GAPGraphviz; object #! @Description Removes an attribute from the object provided. diff --git a/gap/dot.gi b/gap/dot.gi index 994146e..e0e1b29 100644 --- a/gap/dot.gi +++ b/gap/dot.gi @@ -310,6 +310,19 @@ function(x, value) return x; end); +InstallMethod(GraphvizGetAttr, "for a graphviz (di)graph or context and string", +[IsGraphvizObject, IsString], +function(x, key) + if not IsBound(GraphvizAttrs(x).(key)) then + return fail; + fi; + return GraphvizAttrs(x).(key); +end); + +InstallMethod(GraphvizGetAttr, "for a graphviz (di)graph or context and string", +[IsGraphvizObject, IsObject], +{x, key} -> GraphvizGetAttr(x, String(key))); + ############################################################################# # GraphvizAddNode ############################################################################# diff --git a/tst/edge.tst b/tst/edge.tst index a095ee3..76dccff 100644 --- a/tst/edge.tst +++ b/tst/edge.tst @@ -150,7 +150,7 @@ gap> GraphvizRemoveEdges(g, 1, 1);; gap> GraphvizEdges(g); [ ] -# Test setting attributes using the []:= syntax +# Test setting attributes gap> g := GraphvizGraph();; gap> n := GraphvizAddEdge(g, "a", "b");; gap> n["color"] := "red";; @@ -165,11 +165,11 @@ rec( color := "blue", label := "1" ) gap> n[1]; fail -# Test getting attributes using the [] syntax +# Test getting attributes gap> g := GraphvizGraph();; -gap> n := GraphvizAddEdge(g, "a", "b");; -gap> n["color"] := "red";; -gap> n["color"]; +gap> e := GraphvizAddEdge(g, "a", "b");; +gap> GraphvizSetAttr(e, "color", "red");; +gap> GraphvizGetAttr(e, "color"); "red" # Test set label (edge) diff --git a/tst/node.tst b/tst/node.tst index 62f99b5..0df86ab 100644 --- a/tst/node.tst +++ b/tst/node.tst @@ -63,16 +63,16 @@ gap> n := GraphvizAddNode(g, 111); gap> AsString(g); "//dot\ngraph {\n\t111\n}\n" -gap> IsBound(GraphvizAttrs(n).1); -false +gap> GraphvizGetAttr(n, 1); +fail gap> GraphvizSetAttr(n, 1, 2); -gap> GraphvizAttrs(n).1; +gap> GraphvizGetAttr(n, 1); "2" gap> GraphvizRemoveAttr(n, 1); -gap> IsBound(GraphvizAttrs(n).1); -false +gap> GraphvizGetAttr(n, 1); +fail # Test removing a node with a non-string name gap> g := GraphvizGraph();; @@ -81,7 +81,7 @@ gap> GraphvizRemoveNode(g, 111);; gap> GraphvizNodes(g); rec( ) -# Test setting attributes using the []:= syntax +# Test setting attributes gap> g := GraphvizGraph();; gap> n := GraphvizAddNode(g, "a");; gap> GraphvizSetAttr(n, "color", "red");; @@ -94,11 +94,11 @@ gap> GraphvizSetAttr(n, "color", "blue");; gap> GraphvizAttrs(n); rec( color := "blue", label := "1" ) -# Test getting attributes using the [] syntax +# Test getting attributes gap> g := GraphvizGraph();; gap> n := GraphvizAddNode(g, "a");; gap> GraphvizSetAttr(n, "color", "red");; -gap> GraphvizAttrs(n).color; +gap> GraphvizGetAttr(n, "color"); "red" # Test set label (node) From ec44416adb771c15079cc922ade3e9518280e49f Mon Sep 17 00:00:00 2001 From: James Mitchell Date: Thu, 1 Oct 2026 15:49:37 +0100 Subject: [PATCH 4/5] Remove \[\] access from edges --- gap/dot.gd | 14 +------------- gap/dot.gi | 17 ----------------- tst/edge.tst | 16 ++++++++-------- 3 files changed, 9 insertions(+), 38 deletions(-) diff --git a/gap/dot.gd b/gap/dot.gd index 05984a3..1cc8956 100644 --- a/gap/dot.gd +++ b/gap/dot.gd @@ -628,26 +628,14 @@ DeclareOperation("GraphvizSetNodeLabels", #! @EndExampleSession DeclareGlobalFunction("ErrorIfNotValidColor"); -#! @BeginGroup -#! @GroupTitle Getting attributes -#! @Arguments edge, attr -#! @Returns an object -#! @Description -#! Gets the value associated with the attribute attr. -#! @BeginExampleSession -#! @EndExampleSession -DeclareOperation("\[\]", [IsGraphvizEdge, IsObject]); -#! @EndGroup - #! @BeginGroup #! @GroupTitle Setting attributes -#! @Arguments node, attr +#! @Arguments edge, attr #! @Returns an object #! @Description #! Sets the value associated with the attribute attr. #! @BeginExampleSession #! @EndExampleSession -#! @Arguments edge, attr DeclareOperation("\[\]\:\=", [IsGraphvizEdge, IsObject, IsObject]); #! @EndGroup diff --git a/gap/dot.gi b/gap/dot.gi index e0e1b29..d73293e 100644 --- a/gap/dot.gi +++ b/gap/dot.gi @@ -159,23 +159,6 @@ x -> x!.Head); InstallMethod(\=, "for graphviz nodes", [IsGraphvizNode, IsGraphvizNode], IsIdenticalObj); -# Accessing edge attributes - -# TODO Remove this, and just use GraphvizAttr(edge) directly? -InstallMethod(\[\], "for a graphviz edge and a string", -[IsGraphvizEdge, IsString], -function(edge, key) - if IsBound(GraphvizAttrs(edge).(key)) then - return GraphvizAttrs(edge).(key); - fi; - return fail; -end); - -# TODO Remove this, and just use GraphvizAttr(edge) directly? -InstallMethod(\[\], "for a graphviz edge and an object", -[IsGraphvizEdge, IsObject], -{edge, key} -> edge[String(key)]); - # TODO Remove this, and just use GraphvizAttr(edge) directly? InstallMethod(\[\]\:\=, "for a graphviz edge and a string", [IsGraphvizEdge, IsString, IsString], diff --git a/tst/edge.tst b/tst/edge.tst index 76dccff..26da424 100644 --- a/tst/edge.tst +++ b/tst/edge.tst @@ -152,17 +152,17 @@ gap> GraphvizEdges(g); # Test setting attributes gap> g := GraphvizGraph();; -gap> n := GraphvizAddEdge(g, "a", "b");; -gap> n["color"] := "red";; -gap> GraphvizAttrs(n); +gap> e := GraphvizAddEdge(g, "a", "b");; +gap> GraphvizSetAttr(e, "color", "red");; +gap> GraphvizAttrs(e); rec( color := "red" ) -gap> n["label"] := 1;; -gap> GraphvizAttrs(n); +gap> GraphvizSetAttr(e, "label", 1);; +gap> GraphvizAttrs(e); rec( color := "red", label := "1" ) -gap> n["color"] := "blue";; -gap> GraphvizAttrs(n); +gap> GraphvizSetAttr(e, "color", "blue");; +gap> GraphvizAttrs(e); rec( color := "blue", label := "1" ) -gap> n[1]; +gap> GraphvizGetAttr(e, 1); fail # Test getting attributes From 1f3ced13ae719cb6d915d44cbcf239f2a24dbe98 Mon Sep 17 00:00:00 2001 From: James Mitchell Date: Thu, 1 Oct 2026 15:58:04 +0100 Subject: [PATCH 5/5] Remove \[\]:= assignment for edges --- gap/dot.gd | 14 ++------------ gap/dot.gi | 18 ++---------------- 2 files changed, 4 insertions(+), 28 deletions(-) diff --git a/gap/dot.gd b/gap/dot.gd index 1cc8956..d36b448 100644 --- a/gap/dot.gd +++ b/gap/dot.gd @@ -561,7 +561,8 @@ DeclareOperation("GraphvizSetAttr", [IsGraphvizObject, IsObject]); #! @Returns the value of an attribute or fail. #! @Description #! This operation returns the current value of the attribute obtained from -#! String(obj), or fail if there's none. +#! String(obj), or fail if the value of the attribute has not +#! been set. #! @BeginExampleSession #! @EndExampleSession DeclareOperation("GraphvizGetAttr", [IsGraphvizObject, IsObject]); @@ -628,17 +629,6 @@ DeclareOperation("GraphvizSetNodeLabels", #! @EndExampleSession DeclareGlobalFunction("ErrorIfNotValidColor"); -#! @BeginGroup -#! @GroupTitle Setting attributes -#! @Arguments edge, attr -#! @Returns an object -#! @Description -#! Sets the value associated with the attribute attr. -#! @BeginExampleSession -#! @EndExampleSession -DeclareOperation("\[\]\:\=", [IsGraphvizEdge, IsObject, IsObject]); -#! @EndGroup - #! @Arguments graph, node_name #! @Returns a &GAPGraphviz; node object or fail #! @Description diff --git a/gap/dot.gi b/gap/dot.gi index d73293e..bdc53cf 100644 --- a/gap/dot.gi +++ b/gap/dot.gi @@ -159,20 +159,6 @@ x -> x!.Head); InstallMethod(\=, "for graphviz nodes", [IsGraphvizNode, IsGraphvizNode], IsIdenticalObj); -# TODO Remove this, and just use GraphvizAttr(edge) directly? -InstallMethod(\[\]\:\=, "for a graphviz edge and a string", -[IsGraphvizEdge, IsString, IsString], -function(edge, key, val) - GraphvizAttrs(edge).(key) := val; -end); - -# TODO Remove this, and use GraphvizSetAttr(edge) instead? -InstallMethod(\[\]\:\=, "for a graphviz edge and an object", -[IsGraphvizEdge, IsObject, IsObject], -function(edge, key, val) - edge[String(key)] := String(val); -end); - InstallMethod(\=, "for graphviz edges", [IsGraphvizEdge, IsGraphvizEdge], IsIdenticalObj); @@ -236,8 +222,8 @@ end); InstallMethod(GraphvizSetAttr, "for a graphviz node or edge, object, and object", [IsGraphvizNodeOrEdge, IsObject, IsObject], -function(x, name, value) - GraphvizAttrs(x).(String(name)) := String(value); +function(x, key, value) + GraphvizAttrs(x).(String(key)) := String(value); return x; end);