Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Amount of nodes and edges have mistakes when reading adjlist file #6132

Merged
merged 15 commits into from
Nov 1, 2022
Merged

Amount of nodes and edges have mistakes when reading adjlist file #6132

merged 15 commits into from
Nov 1, 2022

Conversation

Qudirah
Copy link
Contributor

@Qudirah Qudirah commented Oct 25, 2022

fixes #5911

@Qudirah
Copy link
Contributor Author

Qudirah commented Oct 27, 2022

@MridulS Please check this and issue #6100 while you are available.

Copy link
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.

Looks good! Thanks

Comment on lines 65 to 69
If spaces are embedded in the names of nodes, the parser in read_adjlist will
be confused because space is the default delimiter. The parser will not
know if a space belongs in the node name or is just a delimiter.
To avoid this problem, specify an alternate delimiter when spaces are
valid in node names.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
If spaces are embedded in the names of nodes, the parser in read_adjlist will
be confused because space is the default delimiter. The parser will not
know if a space belongs in the node name or is just a delimiter.
To avoid this problem, specify an alternate delimiter when spaces are
valid in node names.
The default `delimiter=" "` will result in unexpected results if node names contain
whitespace characters.

Just a minor wording suggestion - I think keeping this as short as possible would be best. Note also that the advice to specify an alternate delimiter may not be possible if the data was generated by someone other than the user.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay. I'll add that and correct it. Thank you @rossbar

@Qudirah
Copy link
Contributor Author

Qudirah commented Nov 1, 2022

@rossbar how about now?

@jarrodmillman jarrodmillman added this to the networkx-2.8.8 milestone Nov 1, 2022
@jarrodmillman jarrodmillman merged commit 858c836 into networkx:main Nov 1, 2022
jarrodmillman pushed a commit that referenced this pull request Nov 1, 2022
)

* fixes #6036

* test load centrality

* test dispersion

* test dispersion

* dispersion test

* test dispersion

* bug-fixes-for-issue-6088

* deleted

* fixes for 5911

* bugfix for 5911
@Qudirah Qudirah deleted the bugfix-for-5911 branch November 2, 2022 09:34
MridulS pushed a commit to MridulS/networkx that referenced this pull request Feb 4, 2023
…tworkx#6132)

* fixes networkx#6036

* test load centrality

* test dispersion

* test dispersion

* dispersion test

* test dispersion

* bug-fixes-for-issue-6088

* deleted

* fixes for 5911

* bugfix for 5911
cvanelteren pushed a commit to cvanelteren/networkx that referenced this pull request Apr 22, 2024
…tworkx#6132)

* fixes networkx#6036

* test load centrality

* test dispersion

* test dispersion

* dispersion test

* test dispersion

* bug-fixes-for-issue-6088

* deleted

* fixes for 5911

* bugfix for 5911
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Amount of nodes and edges have mistakes when reading adjlist file
4 participants