Skip to content

doc: update documentation when providing an iterator over current graph to add/remove_edges_from.#6268

Merged
MridulS merged 16 commits intonetworkx:mainfrom
SultanOrazbayev:doc-add-remove-from-iterator
Dec 12, 2022
Merged

doc: update documentation when providing an iterator over current graph to add/remove_edges_from.#6268
MridulS merged 16 commits intonetworkx:mainfrom
SultanOrazbayev:doc-add-remove-from-iterator

Conversation

@SultanOrazbayev
Copy link
Copy Markdown
Contributor

Resolves #2717

@SultanOrazbayev SultanOrazbayev marked this pull request as ready for review December 9, 2022 15:26
@SultanOrazbayev SultanOrazbayev changed the title doc for add_edges_from doc: update documentation when providing an iterator over current graph to add/remove_edges_from. Dec 10, 2022
Copy link
Copy Markdown
Member

@dschult dschult left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks nice -- and the issue has been around a long time without action... so thanks!

I have wording suggestions -- they are optional, so decide what words you think work best. The start of the notes seems awkward and in the last sentence, the word "variable" isn't quite right since the list often isn't assigned to a variable name.

For the notes sections, perhaps start with:
When adding nodes from an iterator over the graph you are changing, ...

for the last sentence, maybe replace "variable" with "object" in two places in the last sentence.

Thanks!!

@SultanOrazbayev
Copy link
Copy Markdown
Contributor Author

Thank you, @dschult , I think it reads better now! I also hesitated about the variable part, but didn't think of using object, now this makes more sense.

Copy link
Copy Markdown
Member

@dschult dschult left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This looks good!

Copy link
Copy Markdown
Member

@MridulS MridulS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MridulS MridulS merged commit 979d54a into networkx:main Dec 12, 2022
@jarrodmillman jarrodmillman added this to the networkx-3.0 milestone Dec 13, 2022
MridulS pushed a commit to MridulS/networkx that referenced this pull request Feb 4, 2023
…ph to add/remove_edges_from. (networkx#6268)

* doc for add_edges_from

* doc for digraph

* doc for multigraph

* multigraph.add_nodes_from returns keylist

* update docs for graph - edges

* doc update: graph.add_nodes_from

* doc update: graph.remove_nodes_from

* doc update: graph.add_edges_from

* doc update: rewording for graph.add_edges_from

* doc update: graph.add_weighted_edges_from rewording

* doc update: digraph updated as graph

* doc update: digraph minor sync

* doc update: multigraph same as graph

* Update graph.py

* Update digraph.py

* Update multigraph.py
cvanelteren pushed a commit to cvanelteren/networkx that referenced this pull request Apr 22, 2024
…ph to add/remove_edges_from. (networkx#6268)

* doc for add_edges_from

* doc for digraph

* doc for multigraph

* multigraph.add_nodes_from returns keylist

* update docs for graph - edges

* doc update: graph.add_nodes_from

* doc update: graph.remove_nodes_from

* doc update: graph.add_edges_from

* doc update: rewording for graph.add_edges_from

* doc update: graph.add_weighted_edges_from rewording

* doc update: digraph updated as graph

* doc update: digraph minor sync

* doc update: multigraph same as graph

* Update graph.py

* Update digraph.py

* Update multigraph.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

remove_edges_from causing RuntimeError on certain view generators

4 participants