Repository: networkx/networkx

Issue:
Update documentation for planar embedding
Let's update the documentation to make it clear that the `check_planarity` function is the primary interface for the planar embedding tools.  Also, the class `PlanarEmbedding` is tricky to make sure it maintains the planar data structure. People not familiar with those ideas should definitely start with the `check_planarity` function.

See discussion in #5079

Discussion:

Patch:
diff --git a/networkx/algorithms/planarity.py b/networkx/algorithms/planarity.py
index 4d1441efc..8f4b29096 100644
--- a/networkx/algorithms/planarity.py
+++ b/networkx/algorithms/planarity.py
@@ -24,6 +24,18 @@ def check_planarity(G, counterexample=False):
         If the graph is planar `certificate` is a PlanarEmbedding
         otherwise it is a Kuratowski subgraph.
 
+    Examples
+    --------
+    >>> G = nx.Graph([(0, 1), (0, 2)])
+    >>> is_planar, P = nx.check_planarity(G)
+    >>> print(is_planar)
+    True
+
+    When `G` is planar, a `PlanarEmbedding` instance is returned:
+
+    >>> P.get_data()
+    {0: [1, 2], 1: [0], 2: [0]}
+
     Notes
     -----
     A (combinatorial) embedding consists of cyclic orderings of the incident
@@ -716,6 +728,8 @@ class PlanarEmbedding(nx.DiGraph):
     The planar embedding is given by a `combinatorial embedding
     <https://en.wikipedia.org/wiki/Graph_embedding#Combinatorial_embedding>`_.
 
+    .. note:: `check_planarity` is the preferred way to check if a graph is planar.
+
     **Neighbor ordering:**
 
     In comparison to a usual graph structure, the embedding also stores the
@@ -761,6 +775,13 @@ class PlanarEmbedding(nx.DiGraph):
     For a half-edge (u, v) that is orientated such that u is below v then the
     face that belongs to (u, v) is to the right of this half-edge.
 
+    See Also
+    --------
+    is _planar :
+        Preferred way to check if an existing graph is planar.
+    check_planarity :
+        A convenient way to create a `PlanarEmbedding`. If not planar, it returns a subgraph that shows this.
+
     Examples
     --------
 
diff --git a/networkx/algorithms/similarity.py b/networkx/algorithms/similarity.py
index d5f463536..2447a9dce 100644
--- a/networkx/algorithms/similarity.py
+++ b/networkx/algorithms/similarity.py
@@ -745,18 +745,30 @@ def optimize_edit_paths(
         N = len(pending_h)
         # assert Ce.C.shape == (M + N, M + N)
 
-        g_ind = [
-            i
-            for i in range(M)
-            if pending_g[i][:2] == (u, u)
-            or any(pending_g[i][:2] in ((p, u), (u, p)) for p, q in matched_uv)
-        ]
-        h_ind = [
-            j
-            for j in range(N)
-            if pending_h[j][:2] == (v, v)
-            or any(pending_h[j][:2] in ((q, v), (v, q)) for p, q in matched_uv)
-        ]
+        # only attempt to match edges after one node match has been made
+        # this will stop self-edges on the first node being automatically deleted
+        # even when a substitution is the better option
+        if matched_uv:
+            g_ind = [
+                i
+                for i in range(M)
+                if pending_g[i][:2] == (u, u)
+                or any(
+                    pending_g[i][:2] in ((p, u), (u, p), (p, p)) for p, q in matched_uv
+                )
+            ]
+            h_ind = [
+                j
+                for j in range(N)
+                if pending_h[j][:2] == (v, v)
+                or any(
+                    pending_h[j][:2] in ((q, v), (v, q), (q, q)) for p, q in matched_uv
+                )
+            ]
+        else:
+            g_ind = []
+            h_ind = []
+
         m = len(g_ind)
         n = len(h_ind)
 
@@ -782,9 +794,9 @@ def optimize_edit_paths(
                             for p, q in matched_uv
                         ):
                             continue
-                    if g == (u, u):
+                    if g == (u, u) or any(g == (p, p) for p, q in matched_uv):
                         continue
-                    if h == (v, v):
+                    if h == (v, v) or any(h == (q, q) for p, q in matched_uv):
                         continue
                     C[k, l] = inf
 
diff --git a/networkx/algorithms/tree/recognition.py b/networkx/algorithms/tree/recognition.py
index 5fbff544e..52da959b6 100644
--- a/networkx/algorithms/tree/recognition.py
+++ b/networkx/algorithms/tree/recognition.py
@@ -157,6 +157,21 @@ def is_forest(G):
     b : bool
         A boolean that is True if `G` is a forest.
 
+    Raises
+    ------
+    NetworkXPointlessConcept
+        If `G` is empty.
+
+    Examples
+    --------
+    >>> G = nx.Graph()
+    >>> G.add_edges_from([(1, 2), (1, 3), (2, 4), (2, 5)])
+    >>> nx.is_forest(G)
+    True
+    >>> G.add_edge(4, 1)
+    >>> nx.is_forest(G)
+    False
+
     Notes
     -----
     In another convention, a directed forest is known as a *polyforest* and
@@ -198,6 +213,21 @@ def is_tree(G):
     b : bool
         A boolean that is True if `G` is a tree.
 
+    Raises
+    ------
+    NetworkXPointlessConcept
+        If `G` is empty.
+
+    Examples
+    --------
+    >>> G = nx.Graph()
+    >>> G.add_edges_from([(1, 2), (1, 3), (2, 4), (2, 5)])
+    >>> nx.is_tree(G)  # n-1 edges
+    True
+    >>> G.add_edge(3, 4)
+    >>> nx.is_tree(G)  # n edges
+    False
+
     Notes
     -----
     In another convention, a directed tree is known as a *polytree* and then
diff --git a/networkx/utils/decorators.py b/networkx/utils/decorators.py
index e8e4bff55..4e065c50b 100644
--- a/networkx/utils/decorators.py
+++ b/networkx/utils/decorators.py
@@ -464,6 +464,9 @@ class argmap:
         function constructs an object (like a file handle) that requires
         post-processing (like closing).
 
+        Note: try_finally decorators cannot be used to decorate generator
+        functions.
+
     Examples
     --------
     Most of these examples use `@argmap(...)` to apply the decorator to
@@ -606,6 +609,38 @@ class argmap:
             # this code doesn't need to worry about closing the file
             print(file.read())
 
+    Decorators with try_finally = True cannot be used with generator functions,
+    because the `finally` block is evaluated before the generator is exhausted::
+
+        @argmap(open_file, "file", try_finally=True)
+        def file_to_lines(file):
+            for line in file.readlines():
+                yield line
+
+    is equivalent to::
+
+        def file_to_lines_wrapped(file):
+            for line in file.readlines():
+                yield line
+
+        def file_to_lines_wrapper(file):
+            try:
+                file = open_file(file)
+                return file_to_lines_wrapped(file)
+            finally:
+                file.close()
+
+    which behaves similarly to::
+
+        def file_to_lines_whoops(file):
+            file = open_file(file)
+            file.close()
+            for line in file.readlines():
+                yield line
+
+    because the `finally` block of `file_to_lines_wrapper` is executed before
+    the caller has a chance to exhaust the iterator.
+
     Notes
     -----
     An object of this class is callable and intended to be used when
@@ -805,15 +840,8 @@ class argmap:
         argmap._lazy_compile
         """
 
-        if inspect.isgeneratorfunction(f):
-
-            def func(*args, __wrapper=None, **kwargs):
-                yield from argmap._lazy_compile(__wrapper)(*args, **kwargs)
-
-        else:
-
-            def func(*args, __wrapper=None, **kwargs):
-                return argmap._lazy_compile(__wrapper)(*args, **kwargs)
+        def func(*args, __wrapper=None, **kwargs):
+            return argmap._lazy_compile(__wrapper)(*args, **kwargs)
 
         # standard function-wrapping stuff
         func.__name__ = f.__name__
@@ -843,6 +871,14 @@ class argmap:
         # this is used to variously call self.assemble and self.compile
         func.__argmap__ = self
 
+        if hasattr(f, "__argmap__"):
+            func.__is_generator = f.__is_generator
+        else:
+            func.__is_generator = inspect.isgeneratorfunction(f)
+
+        if self._finally and func.__is_generator:
+            raise nx.NetworkXError("argmap cannot decorate generators with try_finally")
+
         return func
 
     __count = 0
@@ -1162,12 +1198,7 @@ class argmap:
         fname = cls._name(f)
         def_sig = f'def {fname}({", ".join(def_sig)}):'
 
-        if inspect.isgeneratorfunction(f):
-            _return = "yield from"
-        else:
-            _return = "return"
-
-        call_sig = f"{_return} {{}}({', '.join(call_sig)})"
+        call_sig = f"return {{}}({', '.join(call_sig)})"
 
         return cls.Signature(fname, sig, def_sig, call_sig, names, npos, args, kwargs)
 


Test patch:
diff --git a/networkx/algorithms/tests/test_similarity.py b/networkx/algorithms/tests/test_similarity.py
index c4fd17f12..9b620dece 100644
--- a/networkx/algorithms/tests/test_similarity.py
+++ b/networkx/algorithms/tests/test_similarity.py
@@ -894,3 +894,27 @@ class TestSimilarity:
 
         assert expected_paths == list(paths)
         assert expected_map == index_map
+
+    def test_symmetry_with_custom_matching(self):
+        print("G2 is edge (a,b) and G3 is edge (a,a)")
+        print("but node order for G2 is (a,b) while for G3 it is (b,a)")
+
+        a, b = "A", "B"
+        G2 = nx.Graph()
+        G2.add_nodes_from((a, b))
+        G2.add_edges_from([(a, b)])
+        G3 = nx.Graph()
+        G3.add_nodes_from((b, a))
+        G3.add_edges_from([(a, a)])
+        for G in (G2, G3):
+            for n in G:
+                G.nodes[n]["attr"] = n
+            for e in G.edges:
+                G.edges[e]["attr"] = e
+        match = lambda x, y: x == y
+
+        print("Starting G2 to G3 GED calculation")
+        assert nx.graph_edit_distance(G2, G3, node_match=match, edge_match=match) == 1
+
+        print("Starting G3 to G2 GED calculation")
+        assert nx.graph_edit_distance(G3, G2, node_match=match, edge_match=match) == 1
diff --git a/networkx/utils/tests/test_decorators.py b/networkx/utils/tests/test_decorators.py
index eee48fd48..3be29e549 100644
--- a/networkx/utils/tests/test_decorators.py
+++ b/networkx/utils/tests/test_decorators.py
@@ -380,29 +380,25 @@ class TestArgmap:
         # context exits are called in reverse
         assert container == ["c", "b", "a"]
 
-    def test_contextmanager_iterator(self):
+    def test_tryfinally_generator(self):
         container = []
 
-        def contextmanager(x):
-            nonlocal container
-            return x, lambda: container.append(x)
+        def singleton(x):
+            return (x,)
 
-        @argmap(contextmanager, 0, 1, 2, try_finally=True)
+        with pytest.raises(nx.NetworkXError):
+
+            @argmap(singleton, 0, 1, 2, try_finally=True)
+            def foo(x, y, z):
+                yield from (x, y, z)
+
+        @argmap(singleton, 0, 1, 2)
         def foo(x, y, z):
-            yield from (x, y, z)
+            return x + y + z
 
         q = foo("a", "b", "c")
-        assert next(q) == "a"
-        assert container == []
-        assert next(q) == "b"
-        assert container == []
-        assert next(q) == "c"
-        assert container == []
-        with pytest.raises(StopIteration):
-            next(q)
 
-        # context exits are called in reverse
-        assert container == ["c", "b", "a"]
+        assert q == ("a", "b", "c")
 
     def test_actual_vararg(self):
         @argmap(lambda x: -x, 4)
@@ -485,7 +481,25 @@ finally:
  pass#"""
         )
 
-    def nop(self):
-        print(foo.__argmap__.assemble(foo.__wrapped__))
-        argmap._lazy_compile(foo)
-        print(foo._code)
+    def test_immediate_raise(self):
+        @not_implemented_for("directed")
+        def yield_nodes(G):
+            yield from G
+
+        G = nx.Graph([(1, 2)])
+        D = nx.DiGraph()
+
+        # test first call (argmap is compiled and executed)
+        with pytest.raises(nx.NetworkXNotImplemented):
+            node_iter = yield_nodes(D)
+
+        # test second call (argmap is only executed)
+        with pytest.raises(nx.NetworkXNotImplemented):
+            node_iter = yield_nodes(D)
+
+        # ensure that generators still make generators
+        node_iter = yield_nodes(G)
+        next(node_iter)
+        next(node_iter)
+        with pytest.raises(StopIteration):
+            next(node_iter)