signature change for `node_link` functions: for issue #5787 (#5899)

* added tests for new signature
 - kept all the original tests.
   Tagged the tests to remove after signature change is complete.

- leveraged a test from test_cyctoscape.py, checks for warnings.

-  made new versions of tests that use attrs in the signature.

* Verified that the new tests fail. Good.

* update signatures with keywords

* added warning message

* adapted code to use keywords

* move test with recwarn fixture outside of class

when recwarn was used in a method, got an error.
work fine in a function.

* add warning to node_link_graph()

* update node_link_graph() to use keywords

* use recwarn in test to dectect warnings

* added deprecation notices to doc strings.

Needs version number for when deprecation will happen

* added example code from doc string to tests

* gave default values to elements of attrs

the example code passes a dict with only some of the keywords defined.
So the defaults needed to be provided

* fixed name conflict

* brace in docstring

* drop braces in doc string example

* Changed namespace to nx in doc string examples

* update the examples

- show the results of the examples
- add more examples of serialization with json

* Update doc desc and notes

- Add reference to Serialization in the method description.
- add a note that the keywords chose for the attributes names must match
   if the two functions are to be used together

* clean up wording

* document the version numbers for deprecating

deprecate the change in 2.8.6
remove the change in 3.1

* Add note, on using old and new keywords together

* Add Deprecation Warning to conftest.py

* Add a reminder to remove code & docs in 3.1

4 files changed, 242 insertions, 44 deletions

diff --git a/doc/developer/deprecations.rst b/doc/developer/deprecations.rst
index b51b0c2f..6871dfca 100644
--- a/doc/developer/deprecations.rst
+++ b/doc/developer/deprecations.rst
@@ -117,6 +117,11 @@ Version 3.0
 * In ``algorithms/matching.py``, remove parameter ``maxcardinality`` from ``min_weight_matching``.
 * In ``drawing/nx_pydot.py``, change PendingDeprecationWarning to DeprecationWarning.
 
+Version 3.1
+~~~~~~~~~~~
+* In ``readwrite/json_graph/node_link.py`` remove the ``attrs` keyword code 
+  and docstring in ``node_link_data`` and ``node_link_graph``. Also the associated tests.
+
 Version 3.2
 ~~~~~~~~~~~
 * In ``generators/directed.py`` remove the ``create_using`` keyword argument
diff --git a/networkx/conftest.py b/networkx/conftest.py
index 20e32e07..7480a5c8 100644
--- a/networkx/conftest.py
+++ b/networkx/conftest.py
@@ -243,6 +243,11 @@ def set_warnings():
     warnings.filterwarnings(
         "ignore", category=PendingDeprecationWarning, message="nx.nx_pydot"
     )
+    warnings.filterwarnings(
+        "ignore",
+        category=DeprecationWarning,
+        message="signature change for node_link functions",
+    )
 
 
 @pytest.fixture(autouse=True)
diff --git a/networkx/readwrite/json_graph/node_link.py b/networkx/readwrite/json_graph/node_link.py
index be186583..419cf05b 100644
--- a/networkx/readwrite/json_graph/node_link.py
+++ b/networkx/readwrite/json_graph/node_link.py
@@ -25,7 +25,9 @@ def _to_tuple(x):
     return tuple(map(_to_tuple, x))
 
 
-def node_link_data(G, attrs=None):
+def node_link_data(
+    G, attrs=None, source="source", target="target", name="id", key="key", link="links"
+):
     """Returns data in node-link format that is suitable for JSON serialization
     and use in Javascript documents.
 
@@ -45,6 +47,27 @@ def node_link_data(G, attrs=None):
         If some user-defined graph data use these attribute names as data keys,
         they may be silently dropped.
 
+        .. deprecated:: 2.8.6
+
+           The `attrs` keyword argument will be replaced with `source`, `target`, `name`,
+           `key` and `link`. in networkx 3.1
+
+           If the `attrs` keyword and the new keywords are both used in a single function call (not recommended)
+           the `attrs` keyword argument will take precedence.
+
+           The values of the keywords must be unique.
+
+    source : string
+        A string that provides the 'source' attribute name for storing NetworkX-internal graph data.
+    target : string
+        A string that provides the 'target' attribute name for storing NetworkX-internal graph data.
+    name : string
+        A string that provides the 'name' attribute name for storing NetworkX-internal graph data.
+    key : string
+        A string that provides the 'key' attribute name for storing NetworkX-internal graph data.
+    link : string
+        A string that provides the 'link' attribute name for storing NetworkX-internal graph data.
+
     Returns
     -------
     data : dict
@@ -53,25 +76,35 @@ def node_link_data(G, attrs=None):
     Raises
     ------
     NetworkXError
-        If values in attrs are not unique.
+        If the values of 'source', 'target' and 'key' are not unique.
 
     Examples
     --------
-    >>> from networkx.readwrite import json_graph
     >>> G = nx.Graph([("A", "B")])
-    >>> data1 = json_graph.node_link_data(G)
-    >>> H = nx.gn_graph(2)
-    >>> data2 = json_graph.node_link_data(
-    ...     H, {"link": "edges", "source": "from", "target": "to"}
-    ... )
+    >>> data1 = nx.node_link_data(G)
+    >>> data1
+    {'directed': False, 'multigraph': False, 'graph': {}, 'nodes': [{'id': 'A'}, {'id': 'B'}], 'links': [{'source': 'A', 'target': 'B'}]}
 
-    To serialize with json
+    To serialize with JSON
 
     >>> import json
     >>> s1 = json.dumps(data1)
-    >>> s2 = json.dumps(
-    ...     data2, default={"link": "edges", "source": "from", "target": "to"}
-    ... )
+    >>> s1
+    '{"directed": false, "multigraph": false, "graph": {}, "nodes": [{"id": "A"}, {"id": "B"}], "links": [{"source": "A", "target": "B"}]}'
+
+    A graph can also be serialized by passing `node_link_data` as an encoder function. The two methods are equivalent.
+
+    >>> s1 = json.dumps(G, default=nx.node_link_data)
+    >>> s1
+    '{"directed": false, "multigraph": false, "graph": {}, "nodes": [{"id": "A"}, {"id": "B"}], "links": [{"source": "A", "target": "B"}]}'
+
+    The attribute names for storing NetworkX-internal graph data can
+    be specified as keyword options.
+
+    >>> H = nx.gn_graph(2)
+    >>> data2 = nx.node_link_data(H, link="edges", source="from", target="to")
+    >>> data2
+    {'directed': True, 'multigraph': False, 'graph': {}, 'nodes': [{'id': 0}, {'id': 1}], 'edges': [{'from': 1, 'to': 0}]}
 
     Notes
     -----
@@ -80,22 +113,43 @@ def node_link_data(G, attrs=None):
 
     Attribute 'key' is only used for multigraphs.
 
+    To use `node_link_data` in conjunction with `node_link_graph`,
+    the keyword names for the attributes must match.
+
+
     See Also
     --------
     node_link_graph, adjacency_data, tree_data
     """
+    # ------ TODO: Remove between the lines after signature change is complete ----- #
+    if attrs is not None:
+        import warnings
+
+        msg = (
+            "\n\nThe `attrs` keyword argument of node_link_data is deprecated\n"
+            "and will be removed in networkx 3.1. It is replaced with explicit\n"
+            "keyword arguments: `source`, `target`, `name`, `key` and `link`.\n"
+            "To make this warning go away, and ensure usage is forward\n"
+            "compatible, replace `attrs` with the keywords. "
+            "For example:\n\n"
+            "   >>> node_link_data(G, attrs={'target': 'foo', 'name': 'bar'})\n\n"
+            "should instead be written as\n\n"
+            "   >>> node_link_data(G, target='foo', name='bar')\n\n"
+            "in networkx 3.1.\n"
+            "The default values of the keywords will not change.\n"
+        )
+        warnings.warn(msg, DeprecationWarning, stacklevel=2)
+
+        source = attrs.get("source", "source")
+        target = attrs.get("target", "target")
+        name = attrs.get("name", "name")
+        key = attrs.get("key", "key")
+        link = attrs.get("link", "links")
+    # -------------------------------------------------- #
     multigraph = G.is_multigraph()
-    # Allow 'attrs' to keep default values.
-    if attrs is None:
-        attrs = _attrs
-    else:
-        attrs.update({k: v for (k, v) in _attrs.items() if k not in attrs})
-    name = attrs["name"]
-    source = attrs["source"]
-    target = attrs["target"]
-    links = attrs["link"]
+
     # Allow 'key' to be omitted from attrs if the graph is not a multigraph.
-    key = None if not multigraph else attrs["key"]
+    key = None if not multigraph else key
     if len({source, target, key}) < 3:
         raise nx.NetworkXError("Attribute names are not unique.")
     data = {
@@ -105,20 +159,31 @@ def node_link_data(G, attrs=None):
         "nodes": [dict(chain(G.nodes[n].items(), [(name, n)])) for n in G],
     }
     if multigraph:
-        data[links] = [
+        data[link] = [
             dict(chain(d.items(), [(source, u), (target, v), (key, k)]))
             for u, v, k, d in G.edges(keys=True, data=True)
         ]
     else:
-        data[links] = [
+        data[link] = [
             dict(chain(d.items(), [(source, u), (target, v)]))
             for u, v, d in G.edges(data=True)
         ]
     return data
 
 
-def node_link_graph(data, directed=False, multigraph=True, attrs=None):
+def node_link_graph(
+    data,
+    directed=False,
+    multigraph=True,
+    attrs=None,
+    source="source",
+    target="target",
+    name="id",
+    key="key",
+    link="links",
+):
     """Returns graph from node-link data format.
+    Useful for de-serialization from JSON.
 
     Parameters
     ----------
@@ -139,6 +204,27 @@ def node_link_graph(data, directed=False, multigraph=True, attrs=None):
             dict(source='source', target='target', name='id',
                 key='key', link='links')
 
+        .. deprecated:: 2.8.6
+
+           The `attrs` keyword argument will be replaced with the individual keywords: `source`, `target`, `name`,
+           `key` and `link`. in networkx 3.1.
+
+           If the `attrs` keyword and the new keywords are both used in a single function call (not recommended)
+           the `attrs` keyword argument will take precedence.
+
+           The values of the keywords must be unique.
+
+    source : string
+        A string that provides the 'source' attribute name for storing NetworkX-internal graph data.
+    target : string
+        A string that provides the 'target' attribute name for storing NetworkX-internal graph data.
+    name : string
+        A string that provides the 'name' attribute name for storing NetworkX-internal graph data.
+    key : string
+        A string that provides the 'key' attribute name for storing NetworkX-internal graph data.
+    link : string
+        A string that provides the 'link' attribute name for storing NetworkX-internal graph data.
+
     Returns
     -------
     G : NetworkX graph
@@ -146,24 +232,65 @@ def node_link_graph(data, directed=False, multigraph=True, attrs=None):
 
     Examples
     --------
-    >>> from networkx.readwrite import json_graph
-    >>> G = nx.Graph([("A", "B")])
-    >>> data = json_graph.node_link_data(G)
-    >>> H = json_graph.node_link_graph(data)
+
+    Create data in node-link format by converting a graph.
+
+    >>> G = nx.Graph([('A', 'B')])
+    >>> data = nx.node_link_data(G)
+    >>> data
+    {'directed': False, 'multigraph': False, 'graph': {}, 'nodes': [{'id': 'A'}, {'id': 'B'}], 'links': [{'source': 'A', 'target': 'B'}]}
+
+    Revert data in node-link format to a graph.
+
+    >>> H = nx.node_link_graph(data)
+    >>> print(H.edges)
+    [('A', 'B')]
+
+    To serialize and deserialize a graph with JSON,
+
+    >>> import json
+    >>> d = json.dumps(node_link_data(G))
+    >>> H = node_link_graph(json.loads(d))
+    >>> print(G.edges, H.edges)
+    [('A', 'B')] [('A', 'B')]
+
 
     Notes
     -----
     Attribute 'key' is only used for multigraphs.
 
+    To use `node_link_data` in conjunction with `node_link_graph`,
+    the keyword names for the attributes must match.
+
     See Also
     --------
     node_link_data, adjacency_data, tree_data
     """
-    # Allow 'attrs' to keep default values.
-    if attrs is None:
-        attrs = _attrs
-    else:
-        attrs.update({k: v for k, v in _attrs.items() if k not in attrs})
+    # ------ TODO: Remove between the lines after signature change is complete ----- #
+    if attrs is not None:
+        import warnings
+
+        msg = (
+            "\n\nThe `attrs` keyword argument of node_link_graph is deprecated\n"
+            "and will be removed in networkx 3.1. It is replaced with explicit\n"
+            "keyword arguments: `source`, `target`, `name`, `key` and `link`.\n"
+            "To make this warning go away, and ensure usage is forward\n"
+            "compatible, replace `attrs` with the keywords. "
+            "For example:\n\n"
+            "   >>> node_link_graph(data, attrs={'target': 'foo', 'name': 'bar'})\n\n"
+            "should instead be written as\n\n"
+            "   >>> node_link_graph(data, target='foo', name='bar')\n\n"
+            "in networkx 3.1.\n"
+            "The default values of the keywords will not change.\n"
+        )
+        warnings.warn(msg, DeprecationWarning, stacklevel=2)
+
+        source = attrs.get("source", "source")
+        target = attrs.get("target", "target")
+        name = attrs.get("name", "name")
+        key = attrs.get("key", "key")
+        link = attrs.get("link", "links")
+    # -------------------------------------------------- #
     multigraph = data.get("multigraph", multigraph)
     directed = data.get("directed", directed)
     if multigraph:
@@ -172,19 +299,16 @@ def node_link_graph(data, directed=False, multigraph=True, attrs=None):
         graph = nx.Graph()
     if directed:
         graph = graph.to_directed()
-    name = attrs["name"]
-    source = attrs["source"]
-    target = attrs["target"]
-    links = attrs["link"]
+
     # Allow 'key' to be omitted from attrs if the graph is not a multigraph.
-    key = None if not multigraph else attrs["key"]
+    key = None if not multigraph else key
     graph.graph = data.get("graph", {})
     c = count()
     for d in data["nodes"]:
         node = _to_tuple(d.get(name, next(c)))
         nodedata = {str(k): v for k, v in d.items() if k != name}
         graph.add_node(node, **nodedata)
-    for d in data[links]:
+    for d in data[link]:
         src = tuple(d[source]) if isinstance(d[source], list) else d[source]
         tgt = tuple(d[target]) if isinstance(d[target], list) else d[target]
         if not multigraph:
diff --git a/networkx/readwrite/json_graph/tests/test_node_link.py b/networkx/readwrite/json_graph/tests/test_node_link.py
index bc46e7ba..28e12ded 100644
--- a/networkx/readwrite/json_graph/tests/test_node_link.py
+++ b/networkx/readwrite/json_graph/tests/test_node_link.py
@@ -6,7 +6,73 @@ import networkx as nx
 from networkx.readwrite.json_graph import node_link_data, node_link_graph
 
 
+# TODO: To be removed when signature change complete
+def test_attrs_deprecation(recwarn):
+    G = nx.path_graph(3)
+
+    # No warnings when `attrs` kwarg not used
+    data = node_link_data(G)
+    H = node_link_graph(data)
+    assert len(recwarn) == 0
+
+    # Future warning raised with `attrs` kwarg
+    attrs = dict(source="source", target="target", name="id", key="key", link="links")
+    data = node_link_data(G, attrs=attrs)
+    assert len(recwarn) == 1
+
+    recwarn.clear()
+    H = node_link_graph(data, attrs=attrs)
+    assert len(recwarn) == 1
+
+
 class TestNodeLink:
+
+    # TODO: To be removed when signature change complete
+    def test_custom_attrs_dep(self):
+        G = nx.path_graph(4)
+        G.add_node(1, color="red")
+        G.add_edge(1, 2, width=7)
+        G.graph[1] = "one"
+        G.graph["foo"] = "bar"
+
+        attrs = dict(
+            source="c_source",
+            target="c_target",
+            name="c_id",
+            key="c_key",
+            link="c_links",
+        )
+
+        H = node_link_graph(
+            node_link_data(G, attrs=attrs), multigraph=False, attrs=attrs
+        )
+        assert nx.is_isomorphic(G, H)
+        assert H.graph["foo"] == "bar"
+        assert H.nodes[1]["color"] == "red"
+        assert H[1][2]["width"] == 7
+
+        # provide only a partial dictionary of keywords.
+        # This is similar to an example in the doc string
+        attrs = dict(
+            link="c_links",
+            source="c_source",
+            target="c_target",
+        )
+        H = node_link_graph(
+            node_link_data(G, attrs=attrs), multigraph=False, attrs=attrs
+        )
+        assert nx.is_isomorphic(G, H)
+        assert H.graph["foo"] == "bar"
+        assert H.nodes[1]["color"] == "red"
+        assert H[1][2]["width"] == 7
+
+    # TODO: To be removed when signature change complete
+    def test_exception_dep(self):
+        with pytest.raises(nx.NetworkXError):
+            G = nx.MultiDiGraph()
+            attrs = dict(name="node", source="node", target="node", key="node")
+            node_link_data(G, attrs)
+
     def test_graph(self):
         G = nx.path_graph(4)
         H = node_link_graph(node_link_data(G))
@@ -68,7 +134,7 @@ class TestNodeLink:
         with pytest.raises(nx.NetworkXError):
             G = nx.MultiDiGraph()
             attrs = dict(name="node", source="node", target="node", key="node")
-            node_link_data(G, attrs)
+            node_link_data(G, **attrs)
 
     def test_string_ids(self):
         q = "qualité"
@@ -97,9 +163,7 @@ class TestNodeLink:
             link="c_links",
         )
 
-        H = node_link_graph(
-            node_link_data(G, attrs=attrs), multigraph=False, attrs=attrs
-        )
+        H = node_link_graph(node_link_data(G, **attrs), multigraph=False, **attrs)
         assert nx.is_isomorphic(G, H)
         assert H.graph["foo"] == "bar"
         assert H.nodes[1]["color"] == "red"