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

Update and test _nx_cugraph._check_networkx_version #24

Open
wants to merge 1 commit into
base: branch-24.12
Choose a base branch
from

Conversation

eriknw
Copy link
Contributor

@eriknw eriknw commented Nov 14, 2024

This fixes handling of the current dev version of networkx "3.5rc0.dev0":
https://github.com/networkx/networkx/blob/5c3e8beef128f532b536d2d4a9f7e309ed53416b/networkx/__init__.py#L11

Also, add testing since this is becoming exercised.

@eriknw eriknw added the non-breaking Introduces a non-breaking change label Nov 14, 2024
@eriknw eriknw requested a review from nv-rliu November 14, 2024 18:58
@eriknw eriknw added the bug Something isn't working label Nov 14, 2024
version_major, version_minor, *version_bug = nx.__version__.split(".")[:3]
if nx_version is None:
nx_version = nx.__version__
version_major, version_minor, *version_bug = nx_version.split(".")[:3]
Copy link
Contributor

@KyleFromNVIDIA KyleFromNVIDIA Nov 18, 2024

Choose a reason for hiding this comment

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

Instead of parsing manually, consider using packaging.version.Version (though this does add a dependency on packaging.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We had been using packaging.version.parse in tests, but this was removed in rapidsai/cugraph#4629 (a very large PR), and other discussions are in rapidsai/cugraph#4571.

What I like about doing this manually is that we have full control and can add functionality and tests whenever there is a regression, b/c although networkx versions are very predictable, sometimes dev versions can be formatted oddly. I expect this to be low-maintenance, and that we will catch issues very promptly.

#22 will also help us use the version tuple correctly.

@eriknw eriknw added this to the 24.12 milestone Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working non-breaking Introduces a non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants