DOC Restructure API documentation - #7798
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughSplit the monolithic API docs into an indexed per-module structure, added an autosummary base template and enabled autosummary generation, updated .gitignore entries, added many per-module API RST files under Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/source/conf.py (1)
78-78:⚠️ Potential issue | 🟡 MinorStale copyright year in Sphinx
copyrightvariable.The SPDX header was updated to
2018-2026, but thecopyrightvariable on Line 78 still reads"2020-2023". This string is rendered in the HTML footer (visible in the PR screenshots as "© Copyright 2020-2023, NVIDIA Corporation"). It should be updated to2026for consistency.Proposed fix
-copyright = "2020-2023, NVIDIA Corporation" +copyright = "2020-2026, NVIDIA Corporation"As per coding guidelines: "Ensure copyright headers of files are up-to-date and in the correct format."
🤖 Fix all issues with AI agents
In `@docs/source/api/cuml.feature_extraction.rst`:
- Around line 1-18: Run codespell against this RST file and fix any spelling
mistakes found in the documentation block for cuml.feature_extraction;
specifically check the module text header and the listed symbols
(CountVectorizer, HashingVectorizer, TfidfVectorizer) for typos and correct them
in the file content so it passes the repository codespell checks.
In `@docs/source/api/cuml.rst`:
- Around line 29-56: The verbosity table under "Verbosity Levels" has
inconsistent sentence punctuation in the "Verbosity level" column; make them
consistent by adding a period at the end of every description entry (so each
bullet for 0, 1, 2, 3, "4 or False", "5 or True", and 6 ends with a period),
updating the list-table rows in docs/source/api/cuml.rst to ensure every
description string terminates with a period.
In `@docs/source/api/cuml.tsa.rst`:
- Around line 16-23: The index.rst entry incorrectly lists the module column as
:mod:`cuml.tsa` for ExponentialSmoothing even though ExponentialSmoothing is
exported at the cuml top level; edit the index.rst line that references
ExponentialSmoothing (the :obj:`~cuml.ExponentialSmoothing` entry) and change
the module column to :mod:`cuml` so the module shown matches the actual export
location (reference symbols: ExponentialSmoothing, cuml, cuml.tsa).
In `@docs/source/api/index.rst`:
- Around line 146-148: Update the Sphinx cross-reference for the ForestInference
entry so it points to the module-specific object path; replace the current
:obj:`~cuml.ForestInference` reference with :obj:`~cuml.fil.ForestInference`
(i.e., change the cross-reference target for ForestInference to
cuml.fil.ForestInference) so the API list uses the same module-qualified form as
other estimators like KMeans or RandomForestClassifier.
🧹 Nitpick comments (5)
docs/source/api/cuml.benchmark.rst (1)
1-20: Different documentation pattern from other module pages.This file uses
automodulewith:members:while most other new module pages (e.g.,cuml.kernel_ridge.rst,cuml.fil.rst,cuml.manifold.rst) use theautosummary+:toctree: generated/+:template: base.rstpattern. If this is intentional (e.g., benchmark utilities don't warrant individual pages), that's fine — just flagging the inconsistency for awareness.docs/source/api/index.rst (2)
9-12: Clarify whether the list-table is meant to be exhaustive or a curated subset.The introductory text says "class and function reference of cuML," suggesting completeness, but the table only lists a subset of items per module (e.g., only 3 of many
cuml.metricsfunctions, no entries forcuml.benchmarkorcuml.experimental). If this is intentionally curated for quick navigation — with full listings available on submodule pages — consider adding a note like "Highlights of the most commonly used classes and functions are listed below" to set expectations.
340-342: Missing trailing newline at end of file.RST files should end with a trailing newline.
docs/source/api/cuml.metrics.rst (2)
6-7: Consider revising the section title for clarity.The section title "Classification and Distance Metrics" combines two distinct concepts. Additionally,
trustworthiness(line 22) is a manifold quality metric rather than a classification or distance metric. Consider splitting this section or using a more accurate title such as "Classification, Probability, and Other Metrics" or simply "Classification Metrics" if the scope is primarily classification-focused.
24-28: Note: Mixed documentation styles across sections.This file uses two different Sphinx directive approaches:
autosummary(explicit listing) for classification and clustering metrics, versusautomodulewith:members:(automatic extraction) for regression metrics, pairwise distances, and pairwise kernels. While this may be intentional based on the module structure, it creates different user experiences when navigating the documentation. Consider documenting the rationale or standardizing the approach if possible.Also applies to: 49-59
Mirroring the classname by using camelcase is super unusual for modules in Python. It creates ambiguity for the references in the docs build. The filenames now match the all lower case style used by all the other modules.
|
Looks like an improvement to me based on the screenshots, but I'm gonna wait for CI to actually build the docs before casting further judgement. 😆 |
|
Yeah, I think seeing it live (not just screenshots) is important. Unfortunately building the project locally has stopped working (downloading CCCL is taking forever). Because there was a change to I'll come back to this on Monday in the hopes that the tubes have gotten unblocked. |
|
We have a rendered preview https://downloads.rapids.ai/ci/cuml/pull-request/7798/bf8fb34/docs/cuml/html/api/ (can be outdated because it contains part of the commit hash) |
| - Description | ||
| - Module | ||
|
|
||
| * - :obj:`~cuml.set_global_output_type` |
There was a problem hiding this comment.
If we add a new function or class to the public API we will have to update this list. I think that is a fine trade-off given how rarely new things get added (and you can ask your AI to do it for you if you want to...). The alternative would be having a script to auto generate this, but I suspect that script would be quite complex to make sure it understands what is public API and what isn't.
Another option is not to have the big table. The main motivation for adding it is that it makes it easy to ctl-f for things and go there directly. Some people reported that they like this about the scikit-learn API reference.
There was a problem hiding this comment.
I think it's perfectly acceptable to manually maintain the main index.
No objection to having at least one big table of all public API names. However, maybe we can split it up into more logical sections?
There was a problem hiding this comment.
The order is alphabetical by sub-module (cuml.accel before cuml.datasets). I'm -0.5 on having subtables/subheadings each time we start a new sub-module.
What sorting/grouping did you have in mind?
There was a problem hiding this comment.
I didn't suggest a new section for each sub-module. I was more thinking along the lines of:
- Configuration and Utility functions
- Zero-code Change Acceleration (cuml.accel)
- Estimators
- Preprocessing
- Model Selection
- Metrics
- Explainability
- Datasets
- Forest Inference (FIL)
- Solvers
- Time series
- Dask (Multi-GPU)
There was a problem hiding this comment.
In 92a6f2e there are now headers and sub-headers that split up the table. I am not sure I like it, but I also don't hate it. I use the API reference the way I use a dictionary: I know the word/class I am looking for, so I often just ctrl-f the page to get to where I want to be. having the headers doesn't really improve things for me. The sub-module name something lives in is the "category" for me
The current headers aren't a perfect copy of the sub-headers of the old page. Can be improved but for now I just wanted to push up a version and see what people think.
csadorf
left a comment
There was a problem hiding this comment.
I think this is going in the right direction, but I'm not a fan that the "main" API index has now lost all of its categorization. In that respect, the current docs are actually more organized.
Group things together based by topic
|
Can we keep the "On this page" TOC on the "API Reference" page? |
|
We can. It uses up some space but not too bad |
|
The API overview page should have the right hand "in this page" TOC now |
80cf210 to
c111cfd
Compare
c111cfd to
f435258
Compare
|
The force pushes were to remove an accidental merge of |
|
/merge |

Closes #7827
The API reference section is now structured a bit more nicely. On
mainit is just one big page and it is hard to see what is what, what is where and who is who.🖼️ Rendered preview https://downloads.rapids.ai/ci/cuml/pull-request/7798/55e9d3b/docs/cuml/html/api/ (can be outdated because it contains part of the commit hash)
With this PR the API reference makes use of subsections for the various modules and maintains a list of "all the things" on that first API page (makes it easy to ctrl-f for a class you know exists and just want to get to fast):
You can navigate into a subsection and see the classes and functions available in it:
And you can visit a particular class/function to read everything there is to know about that class:
There is an issue about restructuring the documentation, and we've discussed it a few times in the past. This morning I was frustrated enough while navigating the API reference section that I did this. I think it is a improvement on the status quo. It does surface that we could be more consistent with how we format our docstrings (eg without a linebreak after the first sentence all of the docstring ends up in the overview table or the fact that some metrics are shown directly on the "cuml.metrics" page but others referred to), etc but that is something we can tackle in a new issue I think.
What do people think? I'm happy to do more polishing and think about HTTP redirects we need and such, but it would be good to know if people agree that this is going in the right direction.