Skip to content

Commit 7f07937

Browse files
author
Release Manager
committed
sagemathgh-41130: Fix G.subgraph(edges=generator) deleting all edges This is a simple fix for a nasty issue: I would expect that the Sage expression `graphs.CompleteGraph(5).subgraph(edges=((0, v) for v in range(5)))` returns a star graph on 4 edges. But it actually returned an edgeless graph! This is because the `subgraph` function expects the `edges` parameter to be a list that can be iterated over twice but the [generator expression](https://peps.python.org/pep-0289/) `((0, v) for v in range(5))` becomes empty after looping over it once, which caused `edges_to_keep_unlabeled` to be empty. While the [documentation for `subgraph()`](https://doc-develop--sagemath .netlify.app/html/en/reference/graphs/sage/graphs/generic_graph.html#sag e.graphs.generic_graph.GenericGraph.subgraph) does not mention that `edges` is allowed to be a generator expression, many other functions that expect an "iterable container" as parameter work just fine if a generator expression is passed instead. For example, `subdivide_edges()`, `delete_edges()` and `G.subgraph(v for v in range(4))` work as expected. There are multiple ways to fix this issue. The simplest and most reliable would be to write `edges = list(edges)`, which would go well with [`vertices = list(vertices)`](https://github.com/sagemath/sage/blob /aa27703384a568d85f1751197efe06ff135be352/src/sage/graphs/generic_graph. py#L14400). Instead I opted to fix the symptom by only iterating over `edges` once, which I believe is faster and uses less memory. URL: sagemath#41130 Reported by: Lennard Hofmann Reviewer(s): David Coudert
2 parents 0ecdc2b + 5b6555d commit 7f07937

File tree

1 file changed

+22
-4
lines changed

1 file changed

+22
-4
lines changed

src/sage/graphs/generic_graph.py

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14378,6 +14378,14 @@ def subgraph(self, vertices=None, edges=None, inplace=False,
1437814378
sage: g.subgraph(list(range(10))) # uses the 'add' algorithm
1437914379
Subgraph of (Path graph): Graph on 10 vertices
1438014380

14381+
The vertices and edges can be specified using generator expressions
14382+
(see :issue:`41130`)::
14383+
14384+
sage: g = graphs.CompleteGraph(5)
14385+
sage: h = g.subgraph(vertices=(v for v in range(4)), edges=((0, v) for v in range(5)))
14386+
sage: h.edges(labels=False)
14387+
[(0, 1), (0, 2), (0, 3)]
14388+
1438114389
TESTS:
1438214390

1438314391
The appropriate properties are preserved::
@@ -14530,8 +14538,13 @@ def _subgraph_by_adding(self, vertices=None, edges=None, edge_property=None, imm
1453014538
G.add_vertices(self if vertices is None else vertices)
1453114539

1453214540
if edges is not None:
14533-
edges_to_keep_labeled = frozenset(e for e in edges if len(e) == 3)
14534-
edges_to_keep_unlabeled = frozenset(e for e in edges if len(e) == 2)
14541+
edges_to_keep_labeled = set()
14542+
edges_to_keep_unlabeled = set()
14543+
for e in edges:
14544+
if len(e) == 3:
14545+
edges_to_keep_labeled.add(e)
14546+
elif len(e) == 2:
14547+
edges_to_keep_unlabeled.add(e)
1453514548

1453614549
edges_to_keep = []
1453714550
if self._directed:
@@ -14712,8 +14725,13 @@ def _subgraph_by_deleting(self, vertices=None, edges=None, inplace=False,
1471214725

1471314726
edges_to_delete = []
1471414727
if edges is not None:
14715-
edges_to_keep_labeled = frozenset(e for e in edges if len(e) == 3)
14716-
edges_to_keep_unlabeled = frozenset(e for e in edges if len(e) == 2)
14728+
edges_to_keep_labeled = set()
14729+
edges_to_keep_unlabeled = set()
14730+
for e in edges:
14731+
if len(e) == 3:
14732+
edges_to_keep_labeled.add(e)
14733+
elif len(e) == 2:
14734+
edges_to_keep_unlabeled.add(e)
1471714735
edges_to_delete = []
1471814736
if G._directed:
1471914737
for e in G.edge_iterator():

0 commit comments

Comments
 (0)