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

Completed Testing in community.py resolves issue #6184 #6185

Merged
merged 5 commits into from
Nov 8, 2022

Conversation

Mjh9122
Copy link
Contributor

@Mjh9122 Mjh9122 commented Nov 7, 2022

Adds testing coverage in community.py. Only branch not tested is the return statement of a nested function, which I do not know how to test. Couple of best practice questions.

  1. When testing internal functions, is it OK to simply call them directly, or should they be covered exclusively by forcing another function to call them?
  2. When forcing an ImportError, what is the best practice, I used sys.modules, but am unsure of other (better) ways.
    Let me know of any other issues
    Testing coverage increase in generators/community.py  #6184

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.

Thanks for this @Mjh9122!

I think this is ready to go, could you just remove the scipy.special test?

@Mjh9122
Copy link
Contributor Author

Mjh9122 commented Nov 8, 2022

@MridulS Just took it out, thanks for your help

Copy link
Contributor

@rossbar rossbar left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @Mjh9122 !

@rossbar rossbar requested a review from MridulS November 8, 2022 21:11
@MridulS MridulS merged commit 3ca5141 into networkx:main Nov 8, 2022
@jarrodmillman jarrodmillman added this to the networkx-3.0 milestone Nov 10, 2022
MridulS pushed a commit to MridulS/networkx that referenced this pull request Feb 4, 2023
…rkx#6185)

* Completed Testing in community.py

* Tried to fix import error

* Tried to fix import error part 2

* Removed scipy test and sys import

* Style
Mjh9122 added a commit to Mjh9122/networkx that referenced this pull request Feb 27, 2023
…rkx#6185)

* Completed Testing in community.py

* Tried to fix import error

* Tried to fix import error part 2

* Removed scipy test and sys import

* Style
cvanelteren pushed a commit to cvanelteren/networkx that referenced this pull request Apr 22, 2024
…rkx#6185)

* Completed Testing in community.py

* Tried to fix import error

* Tried to fix import error part 2

* Removed scipy test and sys import

* Style
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.

None yet

4 participants