Make pylibcugraphops optional imports in cugraph-dgl and -pyg
#3693
Conversation
| def __init__(self): | ||
| super().__init__() |
There was a problem hiding this comment.
Yes, linter thinks this is redundant, as it does not add anything on top of __init__() of base class.
There was a problem hiding this comment.
Gotcha, I am unsure if we should remove the __init__ call for derived classes. Maybe cause I have not seen it.
CC: @rlratzel/ @eriknw , in case we have opinions on calling the constructor of the base class implicitly/explicitly.
There was a problem hiding this comment.
Defining __init__ here does seem unnecessary, since it doesn't change any arguments to __init__ nor does it do anything else at all any more since it no longer checks whether pylibcugraphops.pytorch exists.
There was a problem hiding this comment.
Thanks for the guidance.
I think this PR is ready to merge now.
| - cugraph=23.08.* | ||
| - pylibcugraphops=23.08.* | ||
| - pytorch>=2.0 | ||
| - pytorch-cuda>=11.8 | ||
| - dgl>=1.1.0.cu* |
|
LGTM, lets merge it in. |
There was a problem hiding this comment.
No action for this PR, but I wonder if longer-term we can get this generated like the others from dependencies.yaml, to at least handle the cugraph version updates every release.
There was a problem hiding this comment.
I just updated dependencies.yaml is in 7134e8d.
|
@tingyu66 are you looking at the test failures? Do you need any additional help? |
|
/merge |
This PR reorders the channels to be compatible with other RAPIDS libs. #3693 added two new channels, but they were ordered differently than [other libraries](https://github.com/rapidsai/cudf/blob/branch-23.08/dependencies.yaml#L163-L169). This caused [an error](https://github.com/rapidsai/docker/actions/runs/5591817313/jobs/10223438072?pr=545#step:10:349) in the `conda-merge` script in rapidsai/docker#545. It's also worth noting that docker overhaul requiring the channel ordering being consistent across libraries' `dependencies.yaml` seems very fragile and will undoubtedly cause more issues down the line. Curious if anyone has thoughts on making this more reliable. Authors: - Ray Douglass (https://github.com/raydouglass) Approvers: - AJ Schmidt (https://github.com/ajschmidt8) - Bradley Dice (https://github.com/bdice) URL: #3721
Fixes #3691 .