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

Generalize statistics to disconnected graphs #4149

Closed
wants to merge 1 commit into from

Conversation

matiasmorant
Copy link

No description provided.

@pep8speaks
Copy link

Hello @matiasmorant! Thanks for opening this PR.

Line 297:89: E501 line too long (95 > 88 characters)
Line 354:89: E501 line too long (93 > 88 characters)

Please install and run psf/black.

@dschult
Copy link
Member

dschult commented Aug 12, 2020

This may seem like a simple change, but the impact is hard to ferret out.
Can you add tests to show what you expect to happen. In particular, we need tests that provide (for simple cases) assurance of what the outcome should be when the graph is disconnected. It would be great to have a reference to some published description of this, and where discrepancies of how to do it exist, a note in the docs about the differences and which this method produces.
Thanks!

Base automatically changed from master to main March 4, 2021 18:20
@MridulS
Copy link
Member

MridulS commented Feb 10, 2023

@matiasmorant let us know if you are still interested in working on this, thanks for opening up the PR!

@rossbar
Copy link
Contributor

rossbar commented Feb 11, 2023

This hasn't seen any updates in a while, and IMO still has a lot of work, namely: 1) what do all of the distance measures mean in the context of disconnected graphs, and 2) formalizing that behavior with extensive testing.

@dschult
Copy link
Member

dschult commented Feb 13, 2023

I'm going to close this PR based on this discussion.
We can still post to this if desired and we'll all see it.

@dschult dschult closed this Feb 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

5 participants