Repository: networkx/networkx
Pull request: Add initial_graph parameter to scale_free_graph and deprecate create_using
Fixes #4729 by adding a new keyword argument named `initial_graph` to replace `create_using`. See the reference issue for details. Note that creating a scale-free graph starting from an existing MultiDiGraph is itself not actually tested, but that's a separate issue.

Problem statement:
Graph instance modified in place by `scale_free_graph`
`nx.scale_free_graph` has a `create_using`argument which accepts different types of inputs. When a graph instance is provided, it is used as a starting point for the graph to which more nodes will be added as is clearly described in the docstring.

However, the instance passed in via `create_using` is also modified in place:

```python
>>> G = nx.scale_free_graph(10)
>>> G.nodes()
NodeView((0, 1, 2, 3, 4, 5, 6, 7, 8, 9))
>>> H = nx.scale_free_graph(20, create_using=G)
>>> G.nodes()
NodeView((0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19))
```

This behavior should either be documented, or the `create_using` argument should be copied (when a graph instance) to prevent the in-place modification. I think the second option is more consistent with the other functions in NetworkX, which generally try to avoid in-place modification of inputs.

Hints from issue discussion:
`create_using` is a strange kind of beast (see NXEP3).  I thought we never used it as a starting graph from which to add nodes and edges. Though apparently `scale_free_graph` does that -- (see below). Learn something every day. :}

The standard treatment is that `create_using`, if it is a NetworkX Graph, is cleared before using it to create the graph.  If it is a graph constructor (like `nx.Graph`) then it is called to create the starting graph.

So...  That argument -- `create_using` is always modified when it is an instance.  Maybe NXEP3 can end up fixing that behavior too.
> The standard treatment is that create_using, if it is a NetworkX Graph, is cleared before using it to create the graph

Yes, this is the first instance that I've come across where this *wasn't* the case. In this particular case, I think this may just be the result of imprecise argument naming. `create_using` doesn't really make sense in this context because the graph [must be a `MultiDiGraph`](https://github.com/networkx/networkx/blob/ead0e65bda59862e329f2e6f1da47919c6b07ca9/networkx/generators/directed.py#L251-L258), so the parameter is really only for passing in an initial graph instance to build upon, not selecting a graph type.

I think the intended behavior would be better reflected by a better argument name, e.g. `initial_graph` with behavior like:

```python
if initial_graph is not None:
    G = initial_graph.copy()
else:
    G = nx.MultiDiGraph([(0, 1), (1, 2), (2, 0)])  # This is the default initialization for the current implementation
```
The feature of having a graph to start building a new graph also appears in #4659 and we should probably be consistent in our choice of names.  Should the argument be named `initial_graph` or `seed_graph`?
>  Should the argument be named initial_graph or seed_graph?

I think both names are good and equally clear. I have a slight preference for `initial_graph` only because the term "seed" is so commonly associated with random number generation, but this is very much a nit.

Commit message: Fix formatting.

Patch to review:
diff --git a/doc/developer/deprecations.rst b/doc/developer/deprecations.rst
index a155ab76548..d89a5574127 100644
--- a/doc/developer/deprecations.rst
+++ b/doc/developer/deprecations.rst
@@ -115,3 +115,8 @@ Version 3.0
 * In ``utils/misc.py`` remove ``dict_to_numpy_array1`` and ``dict_to_numpy_array2``.
 * In ``utils/misc.py`` remove ``to_tuple``.
 * In ``algorithms/matching.py``, remove parameter ``maxcardinality`` from ``min_weight_matching``.
+
+Version 3.X
+~~~~~~~~~~~
+* In ``generators/directed.py`` remove the ``create_using`` keyword argument
+  for the ``scale_free_graph`` function.
diff --git a/networkx/conftest.py b/networkx/conftest.py
index 5541b68b6ee..366a99aafde 100644
--- a/networkx/conftest.py
+++ b/networkx/conftest.py
@@ -236,6 +236,10 @@ def set_warnings():
     )
     warnings.filterwarnings("ignore", category=DeprecationWarning, message="info")
     warnings.filterwarnings("ignore", category=DeprecationWarning, message="to_tuple")
+    # create_using for scale_free_graph
+    warnings.filterwarnings(
+        "ignore", category=DeprecationWarning, message="The create_using argument"
+    )
 
 
 @pytest.fixture(autouse=True)
diff --git a/networkx/generators/directed.py b/networkx/generators/directed.py
index d76e79ef2bb..21afab40796 100644
--- a/networkx/generators/directed.py
+++ b/networkx/generators/directed.py
@@ -189,6 +189,7 @@ def scale_free_graph(
     delta_out=0,
     create_using=None,
     seed=None,
+    initial_graph=None,
 ):
     """Returns a scale-free directed graph.
 
@@ -215,9 +216,22 @@ def scale_free_graph(
         The default is a MultiDiGraph 3-cycle.
         If a graph instance, use it without clearing first.
         If a graph constructor, call it to construct an empty graph.
+
+        .. deprecated:: 3.0
+
+           create_using is deprecated, use `initial_graph` instead.
+
     seed : integer, random_state, or None (default)
         Indicator of random number generation state.
         See :ref:`Randomness<randomness>`.
+    initial_graph : MultiDiGraph instance, optional
+        Build the scale-free graph starting from this initial MultiDiGraph,
+        if provided.
+
+
+    Returns
+    -------
+    MultiDiGraph
 
     Examples
     --------
@@ -245,14 +259,37 @@ def _choose_node(candidates, node_list, delta):
                 return seed.choice(node_list)
         return seed.choice(candidates)
 
-    if create_using is None or not hasattr(create_using, "_adj"):
-        # start with 3-cycle
-        G = nx.empty_graph(3, create_using, default=nx.MultiDiGraph)
-        G.add_edges_from([(0, 1), (1, 2), (2, 0)])
-    else:
+    if create_using is not None:
+        import warnings
+
+        warnings.warn(
+            "The create_using argument is deprecated and will be removed in the future.\n\n"
+            "To create a scale free graph from an existing MultiDiGraph, use\n"
+            "initial_graph instead.",
+            DeprecationWarning,
+            stacklevel=2,
+        )
+
+    # TODO: Rm all this complicated logic when deprecation expires and replace
+    # with commented code:
+    #    if initial_graph is not None and hasattr(initial_graph, "_adj"):
+    #        G = initial_graph
+    #    else:
+    #        # Start with 3-cycle
+    #        G = nx.MultiDiGraph([(0, 1), (1, 2), (2, 0)])
+    if create_using is not None and hasattr(create_using, "_adj"):
+        if initial_graph is not None:
+            raise ValueError(
+                "Cannot set both create_using and initial_graph. Set create_using=None."
+            )
         G = create_using
+    else:
+        if initial_graph is not None and hasattr(initial_graph, "_adj"):
+            G = initial_graph
+        else:
+            G = nx.MultiDiGraph([(0, 1), (1, 2), (2, 0)])
     if not (G.is_directed() and G.is_multigraph()):
-        raise nx.NetworkXError("MultiDiGraph required in create_using")
+        raise nx.NetworkXError("MultiDiGraph required in initial_graph")
 
     if alpha <= 0:
         raise ValueError("alpha must be > 0.")
diff --git a/networkx/generators/tests/test_directed.py b/networkx/generators/tests/test_directed.py
index 04981d47b47..66f34c4f244 100644
--- a/networkx/generators/tests/test_directed.py
+++ b/networkx/generators/tests/test_directed.py
@@ -58,6 +58,12 @@ def test_create_using_keyword_arguments(self):
         pytest.raises(ValueError, scale_free_graph, 100, gamma=-0.3)
 
 
+@pytest.mark.parametrize("ig", (nx.Graph(), nx.DiGraph([(0, 1)])))
+def test_scale_free_graph_initial_graph_kwarg(ig):
+    with pytest.raises(nx.NetworkXError):
+        scale_free_graph(100, initial_graph=ig)
+
+
 class TestRandomKOutGraph:
     """Unit tests for the
     :func:`~networkx.generators.directed.random_k_out_graph` function.


Code review comments:
diff_hunk: @@ -115,3 +115,8 @@ Version 3.0
 * In ``utils/misc.py`` remove ``dict_to_numpy_array1`` and ``dict_to_numpy_array2``.
 * In ``utils/misc.py`` remove ``to_tuple``.
 * In ``algorithms/matching.py``, remove parameter ``maxcardinality`` from ``min_weight_matching``.
+
+Version 3.X
path: doc/developer/deprecations.rst
text: @user1:
```suggestion
Version 3.2
```