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

Updates nx-cugraph README for latest h/w, CUDA, python, NX requirements, moves updater to pre-commit #4225

Merged

Conversation

rlratzel
Copy link
Contributor

@rlratzel rlratzel commented Mar 8, 2024

  • Updates nx-cugraph README for latest h/w, CUDA, python, NX requirements
  • Moves the call to the nx-cugraph README check and auto-updater script from the python CI test script to a pre-commit hook alongside the other codegen calls/checks. The pre-commit checks are run by the dev at commit-time and by CI as part of the style check job. This check will only run for changes to files under the nx-cugraph subdir.
  • Updates the update_readme.py script to optionally download the NX objects.inv file itself so calling make is not required as part of the pre-commit hook.
    • NOTE: This is less important though, since the pre-commit hook still calls bash in order to set the PYTHONPATH env var to use the local version of the update_readme.py script, which is what I was trying to avoid in the first place. This still has some advantages in making the update_readme.py script more usable if the user isn't aware of how to obtain a objects.inv file, but feedback is welcome. The hook could also be changed to just call make if that ends up being preferred, in which case we'd have to consider if we still want the updates to update_readme.py to download the objects.inv file itself or not.

Here's the check running as part of the style-check job for this PR:
image

@rlratzel rlratzel added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Mar 8, 2024
@rlratzel rlratzel added this to the 24.04 milestone Mar 8, 2024
@rlratzel rlratzel requested a review from eriknw March 8, 2024 21:00
@rlratzel rlratzel requested review from a team as code owners March 8, 2024 21:00
python/nx-cugraph/Makefile Outdated Show resolved Hide resolved
Copy link
Contributor

@eriknw eriknw left a comment

Choose a reason for hiding this comment

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

Thanks Rick. It's nice to move what we can into pre-commit.

Any appetite for updating linting in pre-commit to better match what we have in nx-cugraph?

Overall looks good, but I shared a few minor comments and nits.

python/nx-cugraph/README.md Outdated Show resolved Hide resolved
ci/test_python.sh Show resolved Hide resolved
python/nx-cugraph/scripts/update_readme.py Outdated Show resolved Hide resolved
…dds NX>=3.2 to pre-commit python environment for nx-cugraph hooks, adds markdown as a type for nx-cugraph README updater, stubs out nx-cugraph runtime deps to allow import without installing them for nx-cugraph pre-commit hooks.
…dds function to produce a string repr with a quoting preference compatible with black, re-generates the _nx_cugraph/__init__.py modified with the wrong quoting preference from the prior commit.
Copy link
Contributor

@acostadon acostadon left a comment

Choose a reason for hiding this comment

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

Looks great.

@rlratzel
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit 79e8e43 into rapidsai:branch-24.04 Mar 13, 2024
142 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci 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.

None yet

5 participants