Remove dictionary from signature of tree_graph and tree_data (#4786)

* Deprecate attrs dict in json_graph.tree fns.

Replace attrs dict in signature with explicit kwargs and
add deprecation warning.

Adds tests that the behavior isn't changed and that warnings are
raised when expected.

* Add deprecations to list and ignore warnings in conftest.

* Modify cytoscape functions attrs deprecation.

* Cleanup cytoscape test suite.

6 files changed, 253 insertions, 158 deletions

diff --git a/doc/developer/deprecations.rst b/doc/developer/deprecations.rst
index 1ceb637b..59e789fc 100644
--- a/doc/developer/deprecations.rst
+++ b/doc/developer/deprecations.rst
@@ -65,6 +65,8 @@ Version 3.0
 * In ``readwrite/json_graph/cytoscape.py``, change function signature for
   ``cytoscape_graph`` and ``cytoscape_data`` to replace the ``attrs`` keyword.
   argument with explicit ``name`` and ``ident`` keyword args.
+* In ``readwrite/json_graph/tree.py``, remove ``attrs`` kwarg from ``tree_graph``
+  and ``tree_data``.
 * Remove ``readwrite/nx_yaml.py`` and related tests.
 * Remove ``readwrite/gpickle.py`` and related tests.
 * Remove ``readwrite/nx_shp.py`` and related tests (add info in alternatives).
diff --git a/networkx/conftest.py b/networkx/conftest.py
index 53bf5dc9..f22d6b14 100644
--- a/networkx/conftest.py
+++ b/networkx/conftest.py
@@ -125,6 +125,14 @@ def set_warnings():
     warnings.filterwarnings(
         "ignore", category=DeprecationWarning, message="iterable is deprecated"
     )
+    warnings.filterwarnings(
+        "ignore",
+        category=FutureWarning,
+        message="\nThe function signature for cytoscape",
+    )
+    warnings.filterwarnings(
+        "ignore", category=DeprecationWarning, message="\nThe `attrs` keyword"
+    )
 
 
 @pytest.fixture(autouse=True)
diff --git a/networkx/readwrite/json_graph/cytoscape.py b/networkx/readwrite/json_graph/cytoscape.py
index 977fd9d2..296242c3 100644
--- a/networkx/readwrite/json_graph/cytoscape.py
+++ b/networkx/readwrite/json_graph/cytoscape.py
@@ -2,11 +2,8 @@ import networkx as nx
 
 __all__ = ["cytoscape_data", "cytoscape_graph"]
 
-# TODO: Remove in NX 3.0
-_attrs = dict(name="name", ident="id")
 
-
-def cytoscape_data(G, attrs=None):
+def cytoscape_data(G, attrs=None, name="name", ident="id"):
     """Returns data in Cytoscape JSON format (cyjs).
 
     Parameters
@@ -24,6 +21,13 @@ def cytoscape_data(G, attrs=None):
            The `attrs` keyword argument will be replaced with `name` and
            `ident` in networkx 3.0
 
+    name : string
+        A string which is mapped to the 'name' node element in cyjs format.
+        Must not have the same value as `ident`.
+    ident : string
+        A string which is mapped to the 'id' node element in cyjs format.
+        Must not have the same value as `name`.
+
     Returns
     -------
     data: dict
@@ -32,7 +36,7 @@ def cytoscape_data(G, attrs=None):
     Raises
     ------
     NetworkXError
-        If the `name` and `ident` attributes are identical.
+        If the values for `name` and `ident` are identical.
 
     See Also
     --------
@@ -55,28 +59,27 @@ def cytoscape_data(G, attrs=None):
       'edges': [{'data': {'source': 0, 'target': 1}}]}}
     """
     # ------ TODO: Remove between the lines in 3.0 ----- #
-    if attrs is None:
-        attrs = _attrs
-    else:
+    if attrs is not None:
         import warnings
 
         msg = (
-            "\nThe function signature for cytoscape_data will change in "
-            "networkx 3.0.\n"
-            "The `attrs` keyword argument will be replaced with \n"
-            "explicit `name` and `ident` keyword arguments, e.g.\n\n"
+            "\nThe `attrs` keyword argument of cytoscape_data is deprecated\n"
+            "and will be removed in networkx 3.0.\n"
+            "It is replaced with explicit `name` and `ident` keyword\n"
+            "arguments.\n"
+            "To make this warning go away and ensure usage is forward\n"
+            "compatible, replace `attrs` with `name` and `ident`,\n"
+            "for example:\n\n"
             "   >>> cytoscape_data(G, attrs={'name': 'foo', 'ident': 'bar'})\n\n"
             "should instead be written as\n\n"
             "   >>> cytoscape_data(G, name='foo', ident='bar')\n\n"
             "in networkx 3.0.\n"
-            "The default values for 'name' and 'ident' will not change."
+            "The default values of 'name' and 'id' will not change."
         )
-        warnings.warn(msg, FutureWarning, stacklevel=2)
-
-        attrs.update({k: v for (k, v) in _attrs.items() if k not in attrs})
+        warnings.warn(msg, DeprecationWarning, stacklevel=2)
 
-    name = attrs["name"]
-    ident = attrs["ident"]
+        name = attrs["name"]
+        ident = attrs["ident"]
     # -------------------------------------------------- #
 
     if name == ident:
@@ -112,7 +115,7 @@ def cytoscape_data(G, attrs=None):
     return jsondata
 
 
-def cytoscape_graph(data, attrs=None):
+def cytoscape_graph(data, attrs=None, name="name", ident="id"):
     """
     Create a NetworkX graph from a dictionary in cytoscape JSON format.
 
@@ -131,6 +134,13 @@ def cytoscape_graph(data, attrs=None):
            The `attrs` keyword argument will be replaced with `name` and
            `ident` in networkx 3.0
 
+    name : string
+        A string which is mapped to the 'name' node element in cyjs format.
+        Must not have the same value as `ident`.
+    ident : string
+        A string which is mapped to the 'id' node element in cyjs format.
+        Must not have the same value as `name`.
+
     Returns
     -------
     graph : a NetworkX graph instance
@@ -172,28 +182,26 @@ def cytoscape_graph(data, attrs=None):
     EdgeDataView([(0, 1, {'source': 0, 'target': 1})])
     """
     # ------ TODO: Remove between the lines in 3.0 ----- #
-    if attrs is None:
-        attrs = _attrs
-    else:
+    if attrs is not None:
         import warnings
 
         msg = (
-            "\nThe function signature for cytoscape_data will change in "
-            "networkx 3.0.\n"
-            "The `attrs` keyword argument will be replaced with \n"
-            "explicit `name` and `ident` keyword arguments, e.g.\n\n"
+            "\nThe `attrs` keyword argument of cytoscape_data is deprecated\n"
+            "and will be removed in networkx 3.0.\n"
+            "It is replaced with explicit `name` and `ident` keyword\n"
+            "arguments.\n"
+            "To make this warning go away and ensure usage is forward\n"
+            "compatible, replace `attrs` with `name` and `ident`,\n"
+            "for example:\n\n"
             "   >>> cytoscape_data(G, attrs={'name': 'foo', 'ident': 'bar'})\n\n"
             "should instead be written as\n\n"
             "   >>> cytoscape_data(G, name='foo', ident='bar')\n\n"
-            "in networkx 3.0.\n"
-            "The default values for 'name' and 'ident' will not change."
+            "The default values of 'name' and 'id' will not change."
         )
-        warnings.warn(msg, FutureWarning, stacklevel=2)
-
-        attrs.update({k: v for (k, v) in _attrs.items() if k not in attrs})
+        warnings.warn(msg, DeprecationWarning, stacklevel=2)
 
-    name = attrs["name"]
-    ident = attrs["ident"]
+        name = attrs["name"]
+        ident = attrs["ident"]
     # -------------------------------------------------- #
 
     if name == ident:
diff --git a/networkx/readwrite/json_graph/tests/test_cytoscape.py b/networkx/readwrite/json_graph/tests/test_cytoscape.py
index 70d3ab4a..1ce84d60 100644
--- a/networkx/readwrite/json_graph/tests/test_cytoscape.py
+++ b/networkx/readwrite/json_graph/tests/test_cytoscape.py
@@ -6,7 +6,7 @@ from networkx.readwrite.json_graph import cytoscape_data, cytoscape_graph
 
 
 # TODO: To be removed when signature change complete in 3.0
-def test_futurewarning():
+def test_attrs_deprecation():
     G = nx.path_graph(3)
     # No warnings when `attrs` kwarg not used
     with pytest.warns(None) as record:
@@ -15,74 +15,78 @@ def test_futurewarning():
     assert len(record) == 0
     # Future warning raised with `attrs` kwarg
     attrs = {"name": "foo", "ident": "bar"}
-    with pytest.warns(FutureWarning):
+    with pytest.warns(DeprecationWarning):
         data = cytoscape_data(G, attrs)
-    with pytest.warns(FutureWarning):
+    with pytest.warns(DeprecationWarning):
         H = cytoscape_graph(data, attrs)
 
 
-class TestCytoscape:
-    def test_graph(self):
-        G = nx.path_graph(4)
-        H = cytoscape_graph(cytoscape_data(G))
-        nx.is_isomorphic(G, H)
-
-    def test_input_data_is_not_modified_when_building_graph(self):
-        G = nx.path_graph(4)
-        input_data = cytoscape_data(G)
-        orig_data = copy.deepcopy(input_data)
-        # Ensure input is unmodified by cytoscape_graph (gh-4173)
-        cytoscape_graph(input_data)
-        assert input_data == orig_data
-
-    def test_graph_attributes(self):
-        G = nx.path_graph(4)
-        G.add_node(1, color="red")
-        G.add_edge(1, 2, width=7)
-        G.graph["foo"] = "bar"
-        G.graph[1] = "one"
-        G.add_node(3, name="node", id="123")
-
-        H = cytoscape_graph(cytoscape_data(G))
-        assert H.graph["foo"] == "bar"
-        assert H.nodes[1]["color"] == "red"
-        assert H[1][2]["width"] == 7
-        assert H.nodes[3]["name"] == "node"
-        assert H.nodes[3]["id"] == "123"
-
-        d = json.dumps(cytoscape_data(G))
-        H = cytoscape_graph(json.loads(d))
-        assert H.graph["foo"] == "bar"
-        assert H.graph[1] == "one"
-        assert H.nodes[1]["color"] == "red"
-        assert H[1][2]["width"] == 7
-        assert H.nodes[3]["name"] == "node"
-        assert H.nodes[3]["id"] == "123"
-
-    def test_digraph(self):
-        G = nx.DiGraph()
-        nx.add_path(G, [1, 2, 3])
-        H = cytoscape_graph(cytoscape_data(G))
-        assert H.is_directed()
-        nx.is_isomorphic(G, H)
-
-    def test_multidigraph(self):
+def test_graph():
+    G = nx.path_graph(4)
+    H = cytoscape_graph(cytoscape_data(G))
+    nx.is_isomorphic(G, H)
+
+
+def test_input_data_is_not_modified_when_building_graph():
+    G = nx.path_graph(4)
+    input_data = cytoscape_data(G)
+    orig_data = copy.deepcopy(input_data)
+    # Ensure input is unmodified by cytoscape_graph (gh-4173)
+    cytoscape_graph(input_data)
+    assert input_data == orig_data
+
+
+def test_graph_attributes():
+    G = nx.path_graph(4)
+    G.add_node(1, color="red")
+    G.add_edge(1, 2, width=7)
+    G.graph["foo"] = "bar"
+    G.graph[1] = "one"
+    G.add_node(3, name="node", id="123")
+
+    H = cytoscape_graph(cytoscape_data(G))
+    assert H.graph["foo"] == "bar"
+    assert H.nodes[1]["color"] == "red"
+    assert H[1][2]["width"] == 7
+    assert H.nodes[3]["name"] == "node"
+    assert H.nodes[3]["id"] == "123"
+
+    d = json.dumps(cytoscape_data(G))
+    H = cytoscape_graph(json.loads(d))
+    assert H.graph["foo"] == "bar"
+    assert H.graph[1] == "one"
+    assert H.nodes[1]["color"] == "red"
+    assert H[1][2]["width"] == 7
+    assert H.nodes[3]["name"] == "node"
+    assert H.nodes[3]["id"] == "123"
+
+
+def test_digraph():
+    G = nx.DiGraph()
+    nx.add_path(G, [1, 2, 3])
+    H = cytoscape_graph(cytoscape_data(G))
+    assert H.is_directed()
+    nx.is_isomorphic(G, H)
+
+
+def test_multidigraph():
+    G = nx.MultiDiGraph()
+    nx.add_path(G, [1, 2, 3])
+    H = cytoscape_graph(cytoscape_data(G))
+    assert H.is_directed()
+    assert H.is_multigraph()
+
+
+def test_multigraph():
+    G = nx.MultiGraph()
+    G.add_edge(1, 2, key="first")
+    G.add_edge(1, 2, key="second", color="blue")
+    H = cytoscape_graph(cytoscape_data(G))
+    assert nx.is_isomorphic(G, H)
+    assert H[1][2]["second"]["color"] == "blue"
+
+
+def test_exception():
+    with pytest.raises(nx.NetworkXError):
         G = nx.MultiDiGraph()
-        nx.add_path(G, [1, 2, 3])
-        H = cytoscape_graph(cytoscape_data(G))
-        assert H.is_directed()
-        assert H.is_multigraph()
-
-    def test_multigraph(self):
-        G = nx.MultiGraph()
-        G.add_edge(1, 2, key="first")
-        G.add_edge(1, 2, key="second", color="blue")
-        H = cytoscape_graph(cytoscape_data(G))
-        assert nx.is_isomorphic(G, H)
-        assert H[1][2]["second"]["color"] == "blue"
-
-    def test_exception(self):
-        with pytest.raises(nx.NetworkXError):
-            G = nx.MultiDiGraph()
-            attrs = dict(name="node", ident="node")
-            cytoscape_data(G, attrs)
+        cytoscape_data(G, name="foo", ident="foo")
diff --git a/networkx/readwrite/json_graph/tests/test_tree.py b/networkx/readwrite/json_graph/tests/test_tree.py
index d6aaf958..1912870f 100644
--- a/networkx/readwrite/json_graph/tests/test_tree.py
+++ b/networkx/readwrite/json_graph/tests/test_tree.py
@@ -4,38 +4,54 @@ import networkx as nx
 from networkx.readwrite.json_graph import tree_data, tree_graph
 
 
-class TestTree:
-    def test_graph(self):
-        G = nx.DiGraph()
-        G.add_nodes_from([1, 2, 3], color="red")
-        G.add_edge(1, 2, foo=7)
-        G.add_edge(1, 3, foo=10)
-        G.add_edge(3, 4, foo=10)
-        H = tree_graph(tree_data(G, 1))
-        nx.is_isomorphic(G, H)
-
-    def test_graph_attributes(self):
-        G = nx.DiGraph()
-        G.add_nodes_from([1, 2, 3], color="red")
-        G.add_edge(1, 2, foo=7)
-        G.add_edge(1, 3, foo=10)
-        G.add_edge(3, 4, foo=10)
-        H = tree_graph(tree_data(G, 1))
-        assert H.nodes[1]["color"] == "red"
-
-        d = json.dumps(tree_data(G, 1))
-        H = tree_graph(json.loads(d))
-        assert H.nodes[1]["color"] == "red"
-
-    def test_exception(self):
-        with pytest.raises(TypeError, match="is not a tree."):
-            G = nx.complete_graph(3)
-            tree_data(G, 0)
-        with pytest.raises(TypeError, match="is not directed."):
-            G = nx.path_graph(3)
-            tree_data(G, 0)
-        with pytest.raises(nx.NetworkXError, match="names are not unique."):
-            G = nx.MultiDiGraph()
-            G.add_node(0)
-            attrs = dict(id="node", children="node")
-            tree_data(G, 0, attrs)
+def test_graph():
+    G = nx.DiGraph()
+    G.add_nodes_from([1, 2, 3], color="red")
+    G.add_edge(1, 2, foo=7)
+    G.add_edge(1, 3, foo=10)
+    G.add_edge(3, 4, foo=10)
+    H = tree_graph(tree_data(G, 1))
+    nx.is_isomorphic(G, H)
+
+
+def test_graph_attributes():
+    G = nx.DiGraph()
+    G.add_nodes_from([1, 2, 3], color="red")
+    G.add_edge(1, 2, foo=7)
+    G.add_edge(1, 3, foo=10)
+    G.add_edge(3, 4, foo=10)
+    H = tree_graph(tree_data(G, 1))
+    assert H.nodes[1]["color"] == "red"
+
+    d = json.dumps(tree_data(G, 1))
+    H = tree_graph(json.loads(d))
+    assert H.nodes[1]["color"] == "red"
+
+
+def test_exceptions():
+    with pytest.raises(TypeError, match="is not a tree."):
+        G = nx.complete_graph(3)
+        tree_data(G, 0)
+    with pytest.raises(TypeError, match="is not directed."):
+        G = nx.path_graph(3)
+        tree_data(G, 0)
+    with pytest.raises(nx.NetworkXError, match="must be different."):
+        G = nx.MultiDiGraph()
+        G.add_node(0)
+        tree_data(G, 0, ident="node", children="node")
+
+
+# NOTE: To be removed when deprecation expires in 3.0
+def test_attrs_deprecation():
+    G = nx.path_graph(3, create_using=nx.DiGraph)
+    # No warnings when `attrs` kwarg not used
+    with pytest.warns(None) as record:
+        data = tree_data(G, 0)
+        H = tree_graph(data)
+    assert len(record) == 0
+    # DeprecationWarning issued when `attrs` is used
+    attrs = {"id": "foo", "children": "bar"}
+    with pytest.warns(DeprecationWarning):
+        data = tree_data(G, 0, attrs=attrs)
+    with pytest.warns(DeprecationWarning):
+        H = tree_graph(data, attrs=attrs)
diff --git a/networkx/readwrite/json_graph/tree.py b/networkx/readwrite/json_graph/tree.py
index 2863712d..2de2ca9b 100644
--- a/networkx/readwrite/json_graph/tree.py
+++ b/networkx/readwrite/json_graph/tree.py
@@ -3,10 +3,9 @@ import networkx as nx
 
 __all__ = ["tree_data", "tree_graph"]
 
-_attrs = dict(id="id", children="children")
 
-
-def tree_data(G, root, attrs=_attrs):
+# NOTE: Remove attrs from signature in 3.0
+def tree_data(G, root, attrs=None, ident="id", children="children"):
     """Returns data in tree format that is suitable for JSON serialization
     and use in Javascript documents.
 
@@ -27,6 +26,19 @@ def tree_data(G, root, attrs=_attrs):
         If some user-defined graph data use these attribute names as data keys,
         they may be silently dropped.
 
+        .. deprecated:: 2.6
+
+           The `attrs` keyword argument is replaced by `ident` and `children`
+           and will be removed in networkx 3.0
+
+    ident : string
+        Attribute name for storing NetworkX-internal graph data. `ident` must
+        have a different value than `children`. The default is 'id'.
+
+    children : string
+        Attribute name for storing NetworkX-internal graph data. `children`
+        must have a different value than `ident`. The default is 'children'.
+
     Returns
     -------
     data : dict
@@ -35,7 +47,7 @@ def tree_data(G, root, attrs=_attrs):
     Raises
     ------
     NetworkXError
-        If values in attrs are not unique.
+        If `children` and `ident` attributes are identical.
 
     Examples
     --------
@@ -55,8 +67,6 @@ def tree_data(G, root, attrs=_attrs):
 
     Graph and edge attributes are not stored.
 
-    The default value of attrs will be changed in a future release of NetworkX.
-
     See Also
     --------
     tree_graph, node_link_data, adjacency_data
@@ -66,10 +76,30 @@ def tree_data(G, root, attrs=_attrs):
     if not G.is_directed():
         raise TypeError("G is not directed.")
 
-    id_ = attrs["id"]
-    children = attrs["children"]
-    if id_ == children:
-        raise nx.NetworkXError("Attribute names are not unique.")
+    # NOTE: to be removed in 3.0
+    if attrs is not None:
+        import warnings
+
+        msg = (
+            "\nThe `attrs` keyword argument of tree_data is deprecated\n"
+            "and will be removed in networkx 3.0.\n"
+            "It is replaced with explicit `ident` and `children` "
+            "keyword arguments.\n"
+            "To make this warning go away and ensure usage is forward\n"
+            "compatible, replace `attrs` with `ident` and `children,\n"
+            "for example:\n\n"
+            "    >>> tree_data(G, root, attrs={'id': 'foo', 'children': 'bar'})\n\n"
+            "should instead be written as\n\n"
+            "    >>> tree_data(G, root, ident='foo', children='bar')\n\n"
+            "The default values of 'id' and 'children' will not change."
+        )
+        warnings.warn(msg, DeprecationWarning, stacklevel=2)
+
+        ident = attrs["id"]
+        children = attrs["children"]
+
+    if ident == children:
+        raise nx.NetworkXError("The values for `id` and `children` must be different.")
 
     def add_children(n, G):
         nbrs = G[n]
@@ -77,19 +107,19 @@ def tree_data(G, root, attrs=_attrs):
             return []
         children_ = []
         for child in nbrs:
-            d = dict(chain(G.nodes[child].items(), [(id_, child)]))
+            d = dict(chain(G.nodes[child].items(), [(ident, child)]))
             c = add_children(child, G)
             if c:
                 d[children] = c
             children_.append(d)
         return children_
 
-    data = dict(chain(G.nodes[root].items(), [(id_, root)]))
+    data = dict(chain(G.nodes[root].items(), [(ident, root)]))
     data[children] = add_children(root, G)
     return data
 
 
-def tree_graph(data, attrs=_attrs):
+def tree_graph(data, attrs=None, ident="id", children="children"):
     """Returns graph from tree data format.
 
     Parameters
@@ -102,6 +132,19 @@ def tree_graph(data, attrs=_attrs):
         NetworkX-internal graph data. The values should be unique. Default
         value: :samp:`dict(id='id', children='children')`.
 
+        .. deprecated:: 2.6
+
+           The `attrs` keyword argument is replaced by `ident` and `children`
+           and will be removed in networkx 3.0
+
+    ident : string
+        Attribute name for storing NetworkX-internal graph data. `ident` must
+        have a different value than `children`. The default is 'id'.
+
+    children : string
+        Attribute name for storing NetworkX-internal graph data. `children`
+        must have a different value than `ident`. The default is 'children'.
+
     Returns
     -------
     G : NetworkX DiGraph
@@ -113,33 +156,47 @@ def tree_graph(data, attrs=_attrs):
     >>> data = json_graph.tree_data(G, root=1)
     >>> H = json_graph.tree_graph(data)
 
-    Notes
-    -----
-    The default value of attrs will be changed in a future release of NetworkX.
-
     See Also
     --------
     tree_data, node_link_data, adjacency_data
     """
     graph = nx.DiGraph()
-    id_ = attrs["id"]
-    children = attrs["children"]
+    if attrs is not None:
+        import warnings
+
+        msg = (
+            "\nThe `attrs` keyword argument of tree_graph is deprecated\n"
+            "and will be removed in networkx 3.0.\n"
+            "It is replaced with explicit `ident` and `children` "
+            "keyword arguments.\n"
+            "To make this warning go away and ensure usage is\n"
+            "forward compatible, replace `attrs` with `ident` and `children,\n"
+            "for example:\n\n"
+            "    >>> tree_graph(data, attrs={'id': 'foo', 'children': 'bar'})\n\n"
+            "should instead be written as\n\n"
+            "    >>> tree_graph(data, ident='foo', children='bar')\n\n"
+            "The default values of 'id' and 'children' will not change."
+        )
+        warnings.warn(msg, DeprecationWarning, stacklevel=2)
+
+        ident = attrs["id"]
+        children = attrs["children"]
 
     def add_children(parent, children_):
         for data in children_:
-            child = data[id_]
+            child = data[ident]
             graph.add_edge(parent, child)
             grandchildren = data.get(children, [])
             if grandchildren:
                 add_children(child, grandchildren)
             nodedata = {
-                str(k): v for k, v in data.items() if k != id_ and k != children
+                str(k): v for k, v in data.items() if k != ident and k != children
             }
             graph.add_node(child, **nodedata)
 
-    root = data[id_]
+    root = data[ident]
     children_ = data.get(children, [])
-    nodedata = {str(k): v for k, v in data.items() if k != id_ and k != children}
+    nodedata = {str(k): v for k, v in data.items() if k != ident and k != children}
     graph.add_node(root, **nodedata)
     add_children(root, children_)
     return graph