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

Add rapids_cython to the html docs #197

Merged

Conversation

robertmaynard
Copy link
Contributor

We forgot to add the rapids_cython function to the API doc page

@robertmaynard robertmaynard added doc Documentation non-breaking Introduces a non-breaking change 3 - Ready for Review Ready for review by team labels May 25, 2022
@robertmaynard robertmaynard requested a review from vyasr May 25, 2022 16:15
Copy link
Contributor

@vyasr vyasr left a comment

Choose a reason for hiding this comment

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

Ah thanks for this, I completely forgot about these pages. I have a couple of typo fixes that I'm just going to push up to your branch, and I suggested adding links to scikit-build if you approve. The only question that I have is, should we document more explicitly that scikit-build is a dependency and that rapids-cython will only work if it's invoked by scikit-build? Since rapids-cmake is developer facing anyway maybe it doesn't really matter since anyone using it should know this.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
docs/api.rst Outdated Show resolved Hide resolved
vyasr and others added 3 commits May 25, 2022 10:43
@robertmaynard
Copy link
Contributor Author

The only question that I have is, should we document more explicitly that scikit-build is a dependency and that rapids-cython will only work if it's invoked by scikit-build?

Yes you should add that to the API paragraph, and also as a ..note: section to rapids_cython_init.

@vyasr
Copy link
Contributor

vyasr commented May 25, 2022

I used a note in both places, let me know if it looks OK to you.

@robertmaynard robertmaynard requested a review from a team as a code owner May 25, 2022 19:56
@robertmaynard
Copy link
Contributor Author

I used a note in both places, let me know if it looks OK to you.

Looks great.

@vyasr
Copy link
Contributor

vyasr commented May 25, 2022

@gpucibot merge

@vyasr vyasr added 5 - DO NOT MERGE Hold off on merging; see PR for details 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 3 - Ready for Review Ready for review by team 5 - DO NOT MERGE Hold off on merging; see PR for details labels May 25, 2022
@vyasr
Copy link
Contributor

vyasr commented May 25, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 50c4bb1 into rapidsai:branch-22.08 May 25, 2022
@robertmaynard robertmaynard deleted the add_rapids_cython_api_index branch June 29, 2022 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to Merge Testing and reviews complete, ready to merge doc Documentation non-breaking Introduces a non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants