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

[REVIEW] Improve Documentation Examples and Source Linking #2541

Merged

Conversation

mdemoret-nv
Copy link
Contributor

Adding some features from sklearn's documentation into cuml. This includes the ability to toggle the output of examples directly in the HTML documentation and adding source links for each class/method.

In order to get the source linking to work, Cython needed to be compiled with binding=True for the compiler directives. See this link for an explanation: https://opendreamkit.org/2017/06/09/CythonSphinx/

Creating this WIP PR to evaluate any performance regressions since binding=True changes how Cython calls pyx functions.

@dantegd Would like to discuss the Cython build process to determine the best way to include binding=True in the current system. I took some inspiration from this Cython issue and its working relatively well.. Would appreciate any feedback since it would be a large change to the build system.

@mdemoret-nv mdemoret-nv requested a review from a team as a code owner July 11, 2020 00:36
@GPUtester
Copy link
Contributor

Please update the changelog in order to start CI tests.

View the gpuCI docs here.

@dantegd dantegd added 2 - In Progress Currenty a work in progress Build or Dep Issues related to building the code or dependencies doc Documentation labels Jul 23, 2020
@mdemoret-nv mdemoret-nv requested a review from a team as a code owner August 7, 2020 05:15
@mdemoret-nv mdemoret-nv requested review from a team as code owners August 26, 2020 22:22
@mdemoret-nv mdemoret-nv changed the base branch from branch-0.15 to branch-0.16 August 26, 2020 22:27
@mdemoret-nv mdemoret-nv changed the title [WIP] Improve Documentation Examples and Source Linking [REVIEW] Improve Documentation Examples and Source Linking Aug 26, 2020
@mdemoret-nv mdemoret-nv removed the 2 - In Progress Currenty a work in progress label Aug 26, 2020
@mdemoret-nv mdemoret-nv added the 3 - Ready for Review Ready for review by team label Aug 26, 2020
@mdemoret-nv
Copy link
Contributor Author

PR is ready to review. Depends on PR #2638 which has been merged into this branch already (so some of the diff files will be due to that PR).

Be sure to look at cuml.metrics.pairwise_distances.pairwise_distances example to see the new toggle button. Many of the examples still use the old style which would be cleaned up in issue #2415.

@dantegd dantegd added the 5 - Merge After Dependencies Depends on another PR: do not merge out of order label Aug 27, 2020
@ajschmidt8 ajschmidt8 removed the request for review from a team August 28, 2020 14:02
@JohnZed JohnZed removed the 5 - Merge After Dependencies Depends on another PR: do not merge out of order label Sep 22, 2020
@JohnZed JohnZed self-assigned this Sep 22, 2020
@mdemoret-nv
Copy link
Contributor Author

@JohnZed This should be good for review pending CI. I added the copyright messages as we discussed to the 3 new files. Not sure if we should add the NV copyright as well. Let me know and I can make the change quickly.

Copy link
Contributor

@JohnZed JohnZed 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! We should get a quick @rapidsai/ops-codeowners signoff as well. It wasn't triggered by these files, but the ops team drives much of the doc publishing, so let's loop them in.

@mdemoret-nv
Copy link
Contributor Author

rerun tests

@dantegd dantegd requested a review from a team September 23, 2020 22:13
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.

confirmed source code link locally. awesome change! 🔥 thanks for the heads up @JohnZed
image

@JohnZed JohnZed changed the title [REVIEW] Improve Documentation Examples and Source Linking [REVIEW] Improve Documentation Examples and Source Linking [skip-ci] Sep 25, 2020
@JohnZed JohnZed changed the title [REVIEW] Improve Documentation Examples and Source Linking [skip-ci] [REVIEW] Improve Documentation Examples and Source Linking Sep 25, 2020
@JohnZed JohnZed merged commit 5300ce4 into rapidsai:branch-0.16 Sep 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team Build or Dep Issues related to building the code or dependencies doc Documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants