Migrate dask-cudf README improvements to dask-cudf sphinx docs #16765
Conversation
rjzamora
added
2 - In Progress
rjzamora
removed
the
improvement
|
@bdice - This PR was partially motivated by your comments/suggestions in #16671 (comment) The primary goal here is to avoid redundant/overlapping documentation in both the dask-cudf readme and the docs. |
rjzamora
added
3 - Ready for Review
rjzamora
added
4 - Needs Review
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
rjzamora
added
5 - Ready to Merge
|
/merge |
| "<div class=\"alert alert-block alert-info\">\n", | ||
| "<b>Note:</b> This notebook uses the explicit Dask cuDF API (dask_cudf) for clarity. However, we strongly recommend that you use Dask's <a href=\"https://docs.dask.org/en/latest/configuration.html\">configuration infrastructure</a> to set the \"dataframe.backend\" option to \"cudf\", and work with the Dask DataFrame API directly. Please see the <a href=\"https://github.com/rapidsai/cudf/tree/main/python/dask_cudf\">Dask cuDF documentation</a> for more information.\n", | ||
| "</div>\n", |
There was a problem hiding this comment.
Do these changes actually format it correctly when viewing the notebook? This is a markdown cell, shouldn't it be formatted as markdown? This is a real question, I have no idea what it should look like and didn't check it myself to how that looks after the changes.
There was a problem hiding this comment.
Yeah, I just made this change after seeing that the cell was "off" in my browser: https://docs.rapids.ai/api/cudf/24.10/user_guide/10min/
I had to do a bit of research to learn that you need to use html to make a note like this work in a jupyter notebook.
python/dask_cudf/README.md
Outdated
| # Compute, persist, or write out the result | ||
| query.compute() |
There was a problem hiding this comment.
Should there be a note/warning like https://github.com/rapidsai/cudf/pull/16765/files#diff-9fe601374fbe50ac48fcee9f76db5d573411078bcd654e7c2bbbbe11abc8d6aaR184-R188 in here? Or possibly even use .persist() only to avoid suggesting practices that aren't often the best.
There was a problem hiding this comment.
compute is typically correct for groupby aggregations, but I'll change to head to be safe. Seem reasonable?
| @@ -115,7 +116,7 @@ | |||
| "source": [ | |||
| "ds = dask_cudf.from_cudf(s, npartitions=2)\n", | |||
| "# Note the call to head here to show the first few entries, unlike\n", | |||
| "# cuDF objects, dask-cuDF objects do not have a printing\n", | |||
| "# cuDF objects, Dask-cuDF objects do not have a printing\n", | |||
There was a problem hiding this comment.
I don't want to annoy you with this @rjzamora , but just wanted to point out in case you missed that you left a few Dask-cuDF entries (with hyphen) and you wanted to explicitly move away from it.
There was a problem hiding this comment.
I think it's fine to hyphenate Dask cuDF when it is used as an adjective.
Description
Follow up to #16671
Checklist