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

Refactor Several MG Tests #4244

Merged
merged 31 commits into from
May 14, 2024

Conversation

nv-rliu
Copy link
Contributor

@nv-rliu nv-rliu commented Mar 15, 2024

Addresses #4187

This PR makes improvements to old MG testing conventions used in these testing directories:

  • centrality
  • comms
  • community
  • components
  • core
  • internals

The goals of this PR is to improve readability and use of the MG tests. Changes are similar to #4197

Instead of using nested fixtures. eg. input_expected_output -> input_combo -> fixture_params which can be confusing, the @pytest.mark.parametrize marker is used to iterate through combinations of parameters used for testing. The fixtures are also replaced by functions used to create SG and MG graphs.

By using the Datasets API (which now supports MG Graphs thanks to @huiyuxie), the number of imports and testing.utils functions can be significantly reduced.

@nv-rliu nv-rliu added this to the 24.06 milestone Mar 15, 2024
@nv-rliu nv-rliu changed the base branch from branch-24.04 to branch-24.06 March 16, 2024 03:29
@nv-rliu nv-rliu added the non-breaking Non-breaking change label Mar 25, 2024
@nv-rliu nv-rliu added the improvement Improvement / enhancement to an existing function label Apr 4, 2024
@nv-rliu nv-rliu marked this pull request as ready for review May 10, 2024 16:36
@nv-rliu nv-rliu requested a review from a team as a code owner May 10, 2024 16:36
@nv-rliu nv-rliu changed the title Refactor MG Tests Refactor Several MG Tests May 10, 2024
@huiyuxie
Copy link
Contributor

Thanks for mentioning me, Ralph! :) I did a rough review and please find it above.

It’s so great to see nice and clean code coming in!

Copy link
Contributor

@rlratzel rlratzel 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 for all the cleanup!

@rlratzel
Copy link
Contributor

/merge

@rapids-bot rapids-bot bot merged commit b9c0fc6 into rapidsai:branch-24.06 May 14, 2024
131 checks passed
@nv-rliu nv-rliu deleted the b2406-mg-test-improvements branch May 15, 2024 04:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement / enhancement to an existing function non-breaking Non-breaking change python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants