fix: add_edges_from drops all edges when given a generator#185
Open
arpitjain099 wants to merge 1 commit into
Open
fix: add_edges_from drops all edges when given a generator#185arpitjain099 wants to merge 1 commit into
arpitjain099 wants to merge 1 commit into
Conversation
RootedDiGraph.add_edges_from iterates ebunch_to_add to register root nodes and build a reconstructed edges list, but then passes the original ebunch_to_add to super().add_edges_from instead of the materialized list. When ebunch_to_add is a single-pass iterable (a generator), the first loop fully consumes it, so the super() call receives an exhausted iterator and adds zero edges. The NetworkX add_edges_from contract accepts an iterable of edge-tuples, which includes generators, so this is a silent correctness bug. The sibling add_nodes_from already passes its materialized list correctly. Pass the already-built edges list to super(), matching add_nodes_from. Add regression tests covering generator input for add_edges_from (2-tuples and 3-tuples with an attr dict) and add_nodes_from. Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
RootedDiGraph.add_edges_fromconsumesebunch_to_addin its own loop to register root nodes, then passes that same argument on tosuper().add_edges_from(...). If a caller hands it a generator, the loop exhausts it and the super call sees nothing, so the graph ends up with zero edges. NetworkX documents the method as accepting any iterable of edge-tuples, generators included. The siblingadd_nodes_fromalready passes its materialized list down, so this just brings the two into line by handing super theedgeslist we already built. It's masked today because the only internal call site passes a re-iterable view.Added regression tests in test/test_graphs.py for generator input (2-tuples and 3-tuples with an attr dict, plus the node case); they fail on the current code and pass with the fix.