Skip to content

Migrated user guide notebooks to MyST-NB and added sphinx extension - #10685

Merged
rapids-bot[bot] merged 4 commits into
NVIDIA:branch-22.06from
mmccarty:myst-nb
Apr 26, 2022
Merged

rapids-bot[bot] merged 4 commits into
NVIDIA:branch-22.06from
mmccarty:myst-nb

Conversation

@mmccarty

@mmccarty mmccarty commented Apr 19, 2022

Copy link
Copy Markdown
Contributor

This PR adds MyST-NB to the sphinx docs build. This will allow the notebooks to be executed when the docs are built to ensure they stay up-to-date with the latest version of the software.

  • Conda env yml updates
  • Sphinx conf.py updates to enable and configure myst-nb
  • Minor fixes to the 10min.ipynb notebook to find test data file path.

@review-notebook-app

Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@github-actions github-actions Bot added the conda label Apr 19, 2022
@codecov

codecov Bot commented Apr 19, 2022

Copy link
Copy Markdown

Codecov Report

Merging #10685 (e1b2d13) into branch-22.06 (01d08af) will increase coverage by 0.04%.
The diff coverage is 98.21%.

❗ Current head e1b2d13 differs from pull request most recent head 14e7574. Consider uploading reports for the commit 14e7574 to get more accurate results

@@               Coverage Diff                @@
##           branch-22.06   #10685      +/-   ##
================================================
+ Coverage         86.35%   86.39%   +0.04%     
================================================
  Files               142      142              
  Lines             22335    22302      -33     
================================================
- Hits              19287    19268      -19     
+ Misses             3048     3034      -14     
Impacted Files Coverage Δ
python/cudf/cudf/core/frame.py 93.41% <ø> (ø)
python/cudf/cudf/utils/utils.py 90.35% <ø> (+0.06%) ⬆️
python/cudf/cudf/core/column/numerical.py 96.17% <50.00%> (+0.29%) ⬆️
python/cudf/cudf/core/column/categorical.py 89.97% <100.00%> (+0.20%) ⬆️
python/cudf/cudf/core/column/column.py 89.43% <100.00%> (-0.02%) ⬇️
python/cudf/cudf/core/column/lists.py 92.91% <100.00%> (+1.39%) ⬆️
python/cudf/cudf/core/column/string.py 89.21% <100.00%> (+0.10%) ⬆️
python/cudf/cudf/core/dataframe.py 93.75% <100.00%> (+<0.01%) ⬆️
python/cudf/cudf/core/groupby/groupby.py 91.68% <100.00%> (+0.27%) ⬆️
python/cudf/cudf/core/multiindex.py 92.28% <100.00%> (+0.13%) ⬆️
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d5e6941...14e7574. Read the comment docs.

@shwina

shwina commented Apr 20, 2022

Copy link
Copy Markdown
Contributor

@mmccarty there's a conflict here due to #10690 modifying guide-to-udfs.ipynb. However this PR deleted that file and replaced it with guide-to-udfs.md.

I incorporated those changes into guide-to-udfs.md as part of my downstream PR #10691 -- you can just steal it from here.

@shwina shwina added non-breaking Non-breaking change doc Documentation labels Apr 20, 2022
@mmccarty
mmccarty marked this pull request as ready for review April 20, 2022 18:20
@mmccarty
mmccarty requested review from a team as code owners April 20, 2022 18:20
@github-actions github-actions Bot added CMake CMake build issue Java Affects Java cuDF API. Python Affects Python cuDF API. gpuCI libcudf Affects libcudf (C++/CUDA) code. and removed Python Affects Python cuDF API. libcudf Affects libcudf (C++/CUDA) code. gpuCI Java Affects Java cuDF API. CMake CMake build issue labels Apr 20, 2022
@shwina

shwina commented Apr 20, 2022

Copy link
Copy Markdown
Contributor

Checked this branch out locally and it looks great! The rendered "notebooks" are also neater than the ones rendered by nbsphinx. I'm +1 to this change.

That being said, there's a downside to moving to Markdown notebooks: the outputs from cell execution aren't included in the source. This means that reviewing significant changes to markdown notebooks is pretty much impossible without checking out the branch locally and running the notebooks manually.

Longer-term, I can imagine CI building docs on every doc-touching PR and providing a way to preview the rendered docs to make review easier.

That would still not be quite as nice as ReviewNB :(

@mmccarty

Copy link
Copy Markdown
Contributor Author

@mmccarty there's a conflict here due to #10690 modifying guide-to-udfs.ipynb. However this PR deleted that file and replaced it with guide-to-udfs.md.

Reverted .ipynb to .md conversion and rebased with branch-22.04 to resolve this conflict.

@shwina shwina left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This LGTM

@ajschmidt8 ajschmidt8 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@mmccarty

Copy link
Copy Markdown
Contributor Author

Will do! Thanks @ajschmidt8

@mmccarty

Copy link
Copy Markdown
Contributor Author

@ajschmidt8 I'm pretty sure those dependencies are only needed in the docs environment. Where we ended up should not impact notebook execution in Jupyter.

@galipremsagar

galipremsagar commented Apr 22, 2022

Copy link
Copy Markdown
Contributor

It looks like ORC section in 10 minutes to CUDF and Dask-CUDF notebook when rendered doesn't have a horizontal scroll and thus overlaps onto the sidebar text like:
Screen Shot 2022-04-22 at 10 42 40 AM

Here is a comparison of nbsphinx:
https://docs.rapids.ai/api/cudf/nightly/user_guide/10min.html#ORC

Same in Converting a CuPy Array to a cuDF DataFrame section and sections below it:
Screen Shot 2022-04-22 at 10 45 26 AM

nbsphinx: https://docs.rapids.ai/api/cudf/nightly/user_guide/10min-cudf-cupy.html#Converting-a-CuPy-Array-to-a-cuDF-DataFrame

Can we get the horizontal scroll with myst-nb too?

@shwina

shwina commented Apr 22, 2022

Copy link
Copy Markdown
Contributor

@galipremsagar looks related to jupyter-book/jupyter-book#1611 perhaps?

@galipremsagar

galipremsagar commented Apr 22, 2022

Copy link
Copy Markdown
Contributor

@galipremsagar looks related to executablebooks/jupyter-book#1611 perhaps?

Yes, to fix it we will have to add the following rule in https://github.com/rapidsai/cudf/blob/branch-22.06/docs/cudf/source/_static/params.css

.output.text_html {
    overflow: auto;
}

Comment thread docs/cudf/source/conf.py
Comment thread docs/cudf/source/_static/params.css Outdated
Comment thread conda/environments/cudf_dev_cuda11.5.yml
Comment thread docs/cudf/source/user_guide/10min.ipynb
@mmccarty

Copy link
Copy Markdown
Contributor Author

rerun tests

@shwina

shwina commented Apr 22, 2022

Copy link
Copy Markdown
Contributor

Looking into the CI failures here.

@shwina

shwina commented Apr 25, 2022

Copy link
Copy Markdown
Contributor

rerun tests

1 similar comment
@shwina

shwina commented Apr 25, 2022

Copy link
Copy Markdown
Contributor

rerun tests

@vyasr vyasr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One minor request to add a comment, otherwise LGTM.

Comment thread docs/cudf/source/_static/params.css
@vyasr

vyasr commented Apr 25, 2022

Copy link
Copy Markdown
Contributor

I committed my proposed change since it looks like this PR is otherwise ready to merge pending tests passing.

@shwina

shwina commented Apr 26, 2022

Copy link
Copy Markdown
Contributor

@gpucibot merge

@rapids-bot
rapids-bot Bot merged commit 93070fa into NVIDIA:branch-22.06 Apr 26, 2022
rapids-bot Bot pushed a commit that referenced this pull request May 13, 2022
Follow-up from #10685 to fix deprecation warnings in the 10 minute notebook.

Fixes: #10613
Changes:
- Fixed deprecation warning for `Series.applymap` ➡️  `Series.apply`
- Removed two cells demonstrating `Series.append`. This has also been removed from the Pandas 10 minute notebook because the feature is deprecated.
- Refactored ORC file path logic to be a bit simpler

Authors:
  - Bradley Dice (https://github.com/bdice)

Approvers:
  - Mike McCarty (https://github.com/mmccarty)
  - GALI PREM SAGAR (https://github.com/galipremsagar)

URL: #10738
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc Documentation non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants