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

Adding a dependencies.yaml file #9

Merged

Conversation

nv-rliu
Copy link
Contributor

@nv-rliu nv-rliu commented Oct 21, 2024

Replaces #8. Accidentally closed the PR trying to revert some changes.

Closes https://github.com/rapidsai/graph_dl/issues/633

This PR creates a dependencies.yaml file for the new nx-cugraph repo

Notes for reviewers:

  • The file was mainly derived from my own understanding of the file, and uses the same naming conventions/packages from cugraph
  • After running rapids-dependency-file-generator the pyproject.toml file was modified

@nv-rliu nv-rliu added feature request New feature or request improvement Improves an existing functionality non-breaking Introduces a non-breaking change and removed feature request New feature or request labels Oct 21, 2024
python/nx-cugraph/pyproject.toml Outdated Show resolved Hide resolved
@bdice
Copy link
Contributor

bdice commented Oct 21, 2024

Is this PR meant to target main? Or branch-24.12? nx-cugraph has a VERSION file saying 24.12.00 but I don't see branch-24.12.

@nv-rliu
Copy link
Contributor Author

nv-rliu commented Oct 22, 2024

Is this PR meant to target main? Or branch-24.12? nx-cugraph has a VERSION file saying 24.12.00 but I don't see branch-24.12.

@bdice I guess this is meant to target 'main' since we don't have a 24.12 branch right now. We haven't gotten to release stages yet. We're still setting up CI on the repo.

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.

LGTM as far as I can tell. Didn't #8 have more changes?

Also, I think the title could be updated.

Finally, I hope you learned not to rewrite history and force-push to a remote repo unless you need to e.g. remove secrets or proprietary info ;)

@nv-rliu nv-rliu changed the title Pre-commit suggestions Adding a dependencies.yaml file Oct 22, 2024
@nv-rliu
Copy link
Contributor Author

nv-rliu commented Oct 22, 2024

LGTM as far as I can tell. Didn't #8 have more changes?

AFAIK #8 also just adds a dependencies file. I think pyproject.toml also gets updating with the pre-commit checks from the new dependencies file. Let me know if you think there are other changes I forgot to take into account.

Also, I think the title could be updated.

Done.

Finally, I hope you learned not to rewrite history and force-push to a remote repo unless you need to e.g. remove secrets or proprietary info ;)

😅

@nv-rliu
Copy link
Contributor Author

nv-rliu commented Oct 23, 2024

Currently waiting on https://github.com/rapidsai/ops/issues/3563 in order to rename branch main to branch-24.12

@nv-rliu
Copy link
Contributor Author

nv-rliu commented Oct 28, 2024

Currently waiting on rapidsai/ops#3563 in order to rename branch main to branch-24.12

These changes have been made to the upstream repo. Addressing feedback and requesting one more review to get this finally merged!

@github-actions github-actions bot added the conda Relates to conda packaging label Oct 28, 2024
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

I pushed a commit with changes from pre-commit run --all-files and this now looks good to me.

Copy link
Member

@ajschmidt8 ajschmidt8 left a comment

Choose a reason for hiding this comment

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

without knowing much about nx-cugraph's dependencies, this PR looks good to me.

@rlratzel rlratzel merged commit 556eed0 into rapidsai:branch-24.12 Oct 28, 2024
3 checks passed
@nv-rliu nv-rliu deleted the branch-24.12-dependencies-file branch October 29, 2024 04:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conda Relates to conda packaging improvement Improves an existing functionality non-breaking Introduces a non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants