Repository: gonum/gonum
Pull request #1643 (2021-05-26 22:42:21)

graph/encoding: provide attribute handling out of the box
This makes node and edge attribute handling much easier for users, in most cases making it possible to just embed an `encoding.Attributes` in the type. For graphs it's a little more involved, for example for dot, an extra method is needed, but it is still simpler than the current situation.

Please take a look.

<!--
Checklist:

- API changes have been discussed
- code is goformated correctly (goimports)
- packages with generated code have had code generation run
- tests pass locally
- linked to relevant issues

Please make sure your commit message summary line and pull request
title match the Go convention; a one-line summary of the change,
prefixed by the primary affected package that should complete the
sentence, "This change modifies Gonum to _____."
-->


kortschak: PTAL
codecov-commenter: # [Codecov](https://codecov.io/gh/gonum/gonum/pull/1643?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gonum) Report
> Merging [#1643](https://codecov.io/gh/gonum/gonum/pull/1643?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gonum) (8b657a8) into [master](https://codecov.io/gh/gonum/gonum/commit/80ff04b367510d857eef0500797782ad56606187?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gonum) (80ff04b) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

> :exclamation: Current head 8b657a8 differs from pull request most recent head 7711e2b. Consider uploading reports for the commit 7711e2b to get more accurate results
[![Impacted file tree graph](https://codecov.io/gh/gonum/gonum/pull/1643/graphs/tree.svg?width=650&height=150&src=pr&token=B7qHnqSPNL&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gonum)](https://codecov.io/gh/gonum/gonum/pull/1643?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gonum)

```diff
@@           Coverage Diff           @@
##           master    #1643   +/-   ##
=======================================
  Coverage   73.21%   73.21%           
=======================================
  Files         503      504    +1     
  Lines       59786    59799   +13     
=======================================
+ Hits        43772    43784   +12     
- Misses      13535    13536    +1     
  Partials     2479     2479           
```


| [Impacted Files](https://codecov.io/gh/gonum/gonum/pull/1643?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gonum) | Coverage Δ | |
|---|---|---|
| [graph/encoding/encoding.go](https://codecov.io/gh/gonum/gonum/pull/1643/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gonum#diff-Z3JhcGgvZW5jb2RpbmcvZW5jb2RpbmcuZ28=) | `100.00% <100.00%> (ø)` | |
| [optimize/listsearch.go](https://codecov.io/gh/gonum/gonum/pull/1643/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gonum#diff-b3B0aW1pemUvbGlzdHNlYXJjaC5nbw==) | `72.22% <0.00%> (-1.86%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/gonum/gonum/pull/1643?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gonum).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gonum)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/gonum/gonum/pull/1643?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gonum). Last update [80ff04b...7711e2b](https://codecov.io/gh/gonum/gonum/pull/1643?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gonum). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gonum).


Patch:
diff --git a/graph/encoding/encoding.go b/graph/encoding/encoding.go
index 53ef0d56..d2037f64 100644
--- a/graph/encoding/encoding.go
+++ b/graph/encoding/encoding.go
@@ -34,3 +34,36 @@ type Attributer interface {
 type Attribute struct {
 	Key, Value string
 }
+
+// Attributes is a helper type providing simple attribute handling.
+type Attributes []Attribute
+
+// Attributes returns all of the receiver's attributes.
+func (a *Attributes) Attributes() []Attribute {
+	return *a
+}
+
+// SetAttribute sets attr in the receiver. Calling SetAttribute with an
+// Attribute with a Key that is in the collection replaces the existing
+// value and calling with an empty Value removes the attribute from the
+// collection if it exists. SetAttribute always returns nil.
+func (a *Attributes) SetAttribute(attr Attribute) error {
+	if attr.Key == "" {
+		return nil
+	}
+	for i, v := range *a {
+		if v.Key == attr.Key {
+			if attr.Value == "" {
+				(*a)[i] = (*a)[len(*a)-1]
+				*a = (*a)[:len(*a)-1]
+				return nil
+			}
+			(*a)[i].Value = attr.Value
+			return nil
+		}
+	}
+	if attr.Value != "" {
+		*a = append(*a, attr)
+	}
+	return nil
+}


Test patch:
diff --git a/graph/encoding/dot/decode_test.go b/graph/encoding/dot/decode_test.go
index 9d1e880f..fe3ee165 100644
--- a/graph/encoding/dot/decode_test.go
+++ b/graph/encoding/dot/decode_test.go
@@ -532,7 +532,13 @@ type dotDirectedGraph struct {
 // newDotDirectedGraph returns a new directed capable of creating user-defined
 // nodes and edges.
 func newDotDirectedGraph() *dotDirectedGraph {
-	return &dotDirectedGraph{DirectedGraph: simple.NewDirectedGraph()}
+	return &dotDirectedGraph{
+		DirectedGraph: simple.NewDirectedGraph(),
+
+		graph: &encoding.Attributes{},
+		node:  &encoding.Attributes{},
+		edge:  &encoding.Attributes{},
+	}
 }
 
 // NewNode returns a new node with a unique node ID for the graph.
@@ -552,7 +558,7 @@ func (g *dotDirectedGraph) DOTAttributers() (graph, node, edge encoding.Attribut
 
 // DOTAttributeSetters implements the dot.AttributeSetters interface.
 func (g *dotDirectedGraph) DOTAttributeSetters() (graph, node, edge encoding.AttributeSetter) {
-	return &g.graph, &g.node, &g.edge
+	return g.graph, g.node, g.edge
 }
 
 // SetDOTID sets the DOT ID of the graph.
@@ -579,7 +585,13 @@ type dotUndirectedGraph struct {
 // newDotUndirectedGraph returns a new undirected capable of creating user-
 // defined nodes and edges.
 func newDotUndirectedGraph() *dotUndirectedGraph {
-	return &dotUndirectedGraph{UndirectedGraph: simple.NewUndirectedGraph()}
+	return &dotUndirectedGraph{
+		UndirectedGraph: simple.NewUndirectedGraph(),
+
+		graph: &encoding.Attributes{},
+		node:  &encoding.Attributes{},
+		edge:  &encoding.Attributes{},
+	}
 }
 
 // NewNode adds a new node with a unique node ID to the graph.
@@ -599,7 +611,7 @@ func (g *dotUndirectedGraph) DOTAttributers() (graph, node, edge encoding.Attrib
 
 // DOTUnmarshalerAttrs implements the dot.UnmarshalerAttrs interface.
 func (g *dotUndirectedGraph) DOTAttributeSetters() (graph, node, edge encoding.AttributeSetter) {
-	return &g.graph, &g.node, &g.edge
+	return g.graph, g.node, g.edge
 }
 
 // SetDOTID sets the DOT ID of the graph.
@@ -705,15 +717,9 @@ func (e *dotEdge) ToPort() (port, compass string) {
 	return e.ToPortLabels.Port, e.ToPortLabels.Compass
 }
 
-// attributes is a helper for global attributes.
-type attributes []encoding.Attribute
-
-func (a attributes) Attributes() []encoding.Attribute {
-	return []encoding.Attribute(a)
-}
-func (a *attributes) SetAttribute(attr encoding.Attribute) error {
-	*a = append(*a, attr)
-	return nil
+type attributes interface {
+	encoding.Attributer
+	encoding.AttributeSetter
 }
 
 const undirectedSelfLoopGraph = `graph {
diff --git a/graph/encoding/dot/encode_test.go b/graph/encoding/dot/encode_test.go
index 8c6f51a0..afec9e94 100644
--- a/graph/encoding/dot/encode_test.go
+++ b/graph/encoding/dot/encode_test.go
@@ -321,10 +321,6 @@ type graphAttributer struct {
 	edge  encoding.Attributer
 }
 
-type attributer []encoding.Attribute
-
-func (a attributer) Attributes() []encoding.Attribute { return a }
-
 func (g graphAttributer) DOTAttributers() (graph, node, edge encoding.Attributer) {
 	return g.graph, g.node, g.edge
 }
@@ -1192,8 +1188,8 @@ var encodeTests = []struct {
 				{from: 2, to: 4}: {},
 				{from: 3, to: 4}: {{Key: "color", Value: "red"}},
 			}),
-			graph: attributer{{Key: "rankdir", Value: `"LR"`}},
-			node:  attributer{{Key: "fontsize", Value: "16"}, {Key: "shape", Value: "ellipse"}},
+			graph: &encoding.Attributes{{Key: "rankdir", Value: `"LR"`}},
+			node:  &encoding.Attributes{{Key: "fontsize", Value: "16"}, {Key: "shape", Value: "ellipse"}},
 			edge:  nil,
 		},
 
diff --git a/graph/encoding/encoding_test.go b/graph/encoding/encoding_test.go
new file mode 100644
index 00000000..4aed09f0
--- /dev/null
+++ b/graph/encoding/encoding_test.go
@@ -0,0 +1,121 @@
+// Copyright ©2021 The Gonum Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package encoding
+
+import (
+	"sort"
+	"testing"
+)
+
+var setAttributesTests = []struct {
+	attr   *Attributes
+	opName string
+	op     func(AttributeSetter) error
+	want   *Attributes
+}{
+	{
+		attr:   &Attributes{},
+		opName: "noop",
+		op: func(a AttributeSetter) error {
+			return a.SetAttribute(Attribute{Key: "", Value: "bar"})
+		},
+		want: &Attributes{},
+	},
+	{
+		attr:   &Attributes{},
+		opName: "add attr to empty",
+		op: func(a AttributeSetter) error {
+			return a.SetAttribute(Attribute{Key: "foo", Value: "bar"})
+		},
+		want: &Attributes{{Key: "foo", Value: "bar"}},
+	},
+	{
+		attr:   &Attributes{},
+		opName: "remove attr from empty",
+		op: func(a AttributeSetter) error {
+			return a.SetAttribute(Attribute{Key: "foo", Value: ""})
+		},
+		want: &Attributes{},
+	},
+	{
+		attr:   &Attributes{{Key: "foo", Value: "bar"}},
+		opName: "add attr to non-empty",
+		op: func(a AttributeSetter) error {
+			return a.SetAttribute(Attribute{Key: "bif", Value: "fud"})
+		},
+		want: &Attributes{{Key: "foo", Value: "bar"}, {Key: "bif", Value: "fud"}},
+	},
+	{
+		attr:   &Attributes{{Key: "foo", Value: "bar"}},
+		opName: "remove attr from singleton",
+		op: func(a AttributeSetter) error {
+			return a.SetAttribute(Attribute{Key: "foo", Value: ""})
+		},
+		want: &Attributes{},
+	},
+	{
+		attr:   &Attributes{{Key: "foo", Value: "bar"}, {Key: "bif", Value: "fud"}},
+		opName: "remove first attr from pair",
+		op: func(a AttributeSetter) error {
+			return a.SetAttribute(Attribute{Key: "foo", Value: ""})
+		},
+		want: &Attributes{{Key: "bif", Value: "fud"}},
+	},
+	{
+		attr:   &Attributes{{Key: "foo", Value: "bar"}, {Key: "bif", Value: "fud"}},
+		opName: "remove second attr from pair",
+		op: func(a AttributeSetter) error {
+			return a.SetAttribute(Attribute{Key: "bif", Value: ""})
+		},
+		want: &Attributes{{Key: "foo", Value: "bar"}},
+	},
+	{
+		attr:   &Attributes{{Key: "foo", Value: "bar"}, {Key: "bif", Value: "fud"}},
+		opName: "replace first attr in pair",
+		op: func(a AttributeSetter) error {
+			return a.SetAttribute(Attribute{Key: "foo", Value: "not bar"})
+		},
+		want: &Attributes{{Key: "foo", Value: "not bar"}, {Key: "bif", Value: "fud"}},
+	},
+	{
+		attr:   &Attributes{{Key: "foo", Value: "bar"}, {Key: "bif", Value: "fud"}},
+		opName: "replace second attr in pair",
+		op: func(a AttributeSetter) error {
+			return a.SetAttribute(Attribute{Key: "bif", Value: "not fud"})
+		},
+		want: &Attributes{{Key: "foo", Value: "bar"}, {Key: "bif", Value: "not fud"}},
+	},
+}
+
+func TestSetAttributes(t *testing.T) {
+	for _, test := range setAttributesTests {
+		err := test.op(test.attr)
+		if err != nil {
+			t.Errorf("unexpected error for %q: %v", test.opName, err)
+			continue
+		}
+		if !sameAttributes(test.attr, test.want) {
+			t.Errorf("unexpected result from %q:\ngot: %+v\nwant:%+v", test.opName, test.attr, test.want)
+		}
+	}
+}
+
+func sameAttributes(a, b Attributer) bool {
+	aAttr := a.Attributes()
+	bAttr := b.Attributes()
+	if len(aAttr) != len(bAttr) {
+		return false
+	}
+	aAttr = append(aAttr[:0:0], aAttr...)
+	sort.Slice(aAttr, func(i, j int) bool { return aAttr[i].Key < aAttr[j].Key })
+	bAttr = append(bAttr[:0:0], bAttr...)
+	sort.Slice(bAttr, func(i, j int) bool { return bAttr[i].Key < bAttr[j].Key })
+	for i, a := range aAttr {
+		if bAttr[i] != a {
+			return false
+		}
+	}
+	return true
+}