[Docs][KubeRay] Add guide for writing KubeRay doctests#51708
[Docs][KubeRay] Add guide for writing KubeRay doctests#51708MortalHappiness wants to merge 5 commits intoray-project:masterfrom
Conversation
MortalHappiness
force-pushed
the
docs/kuberay-doctests-guide
branch
from
0af5bd0 to
2032b7e
Compare
|
cc @andrewsykim |
|
cc @rueian |
Signed-off-by: Chi-Sheng Liu <chishengliu@chishengliu.com>
MortalHappiness
force-pushed
the
docs/kuberay-doctests-guide
branch
from
2032b7e to
1f2f0d3
Compare
MortalHappiness
force-pushed
the
docs/kuberay-doctests-guide
branch
from
8a591b4 to
33fa5a6
Compare
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.
|
stale
bot
added
the
stale
|
@MortalHappiness can you ask a contributor who has contributed to doctest to review this PR? Thanks! |
stale
bot
removed
the
stale
|
@JiangJiaWei1103 @popojk @kenchung285 @machichima Please help review this PR. Thanks. |
Signed-off-by: Chi-Sheng Liu <chishengliu@chishengliu.com>
|
LGTM |
|
I have already requested the doc team to review. |
| checkpoint_path = train_ppo_model() | ||
| ``` | ||
|
|
||
| (jupyter-notebook-tags)= |
There was a problem hiding this comment.
is this supposed to be here?
There was a problem hiding this comment.
Yes. It is referenced here.
|
@angelinalg should probably review this one too though since it's a guide on how to write docs |
Co-authored-by: Dhyey Shah <dhyey2019@gmail.com> Signed-off-by: Chi-Sheng Liu <chishengliu@chishengliu.com>
|
This pull request has been automatically marked as stale because it has not had You can always ask for help on our discussion forum or Ray's public slack channel. If you'd like to keep this open, just leave any comment, and the stale label will be removed. |
github-actions
bot
added
the
stale
|
not stale |
|
I have already requested the doc team to review. |
github-actions
bot
removed
the
stale
dstrodtman
left a comment
dstrodtman
left a comment
There was a problem hiding this comment.
@kevin85421 Made some suggestions that should improve the usability of the page.
The main thing here: as this is currently written, this is an example, but needs some rework to make it easier to separate out the steps to be completed and the explanation of those steps.
Happy to discuss if you'd like!
|
|
||
| ## Writing docs | ||
|
|
||
| ```shell |
There was a problem hiding this comment.
Add a sentence to tell users what this code is doing.
|
|
||
| ## Prerequisites | ||
|
|
||
| ```shell |
There was a problem hiding this comment.
Add a sentence to tell users what this code is doing.
There was a problem hiding this comment.
I think you want to explain here what all these requirements are, and then show this in the next section alongside the command to CD into the docs and launch Jupyter lab?
Or is this running as part of an image definition?
| @@ -0,0 +1,59 @@ | |||
| # How to write KubeRay doctests | |||
|
|
|||
| KubeRay doctests require you to write the doc using a Jupyter notebook with the Bash kernel. | |||
There was a problem hiding this comment.
| KubeRay doctests require you to write the doc using a Jupyter notebook with the Bash kernel. | |
| This page provides an overview of writing doctests for KubeRay for submissions to documentation. | |
| You must write your doc using a Jupyter notebook with the Bash kernel to use doctests with KubeRay. |
Don't start with a requirement. Always tell the reader what they will learn from the page.
|
|
||
|  | ||
|
|
||
| Write the doc using a Jupyter notebook. Make sure you select the Bash kernel (red box in the preceding image). |
There was a problem hiding this comment.
Don't use colors (color blindness/accessibility), and don't refer to a preceding image.
Redo the image to use numbers. I think something like:
| Write the doc using a Jupyter notebook. Make sure you select the Bash kernel (red box in the preceding image). | |
| In the Jupyter notebook where you are writing your doc, do the following: | |
| 1. Select the Bash kernel. | |
| 2. Use **Common Tools** in the side panel to manage tags for code blocks. | |
|  |
|
|
||
| Other tags control whether the code block runs during the doctest. See [nbval documentation][nbval-doc] for details. | ||
|
|
||
| For example, the `kind create cluster` code block in the image above uses the `nbval-ignore-output` and `remove-output` tags. `remove-output` hides the block's output in the final document. `nbval-ignore-output` skips output validation during the doctest run. |
There was a problem hiding this comment.
Looking at this, I realize you might want to fully rewrite your steps to work as a tutorial. You could then give instruction to
#. Copy the following code into a cell: kind create cluster.
#. Select the cell and click Common Tools in the side panel.
#. Add the tags nbval-ignore-output and remove-output.
#. Select Restart Kernel and Run All Cells....
Basically, rewrite this whole section as a step-by-step example that anyone can complete, explain what the example does, and then hope that users can generalize this (you're doing this now, but interspersing explanation with steps makes it more difficult to follow both the steps and the explanation of what is being accomplished)
| py.test --nbval getting-started/raycluster-quick-start.ipynb --nbval-kernel-name bash --sanitize-with doc_sanitize.cfg | ||
| ``` | ||
|
|
||
| The `doc/source/cluster/kubernetes/doc_sanitize.cfg` file lists patterns to replace in the code block output before comparing it with the saved output. This helps when the output includes dynamic values like timestamps. If the output changes each time you run the doctest, add the relevant patterns to this file. |
There was a problem hiding this comment.
| The `doc/source/cluster/kubernetes/doc_sanitize.cfg` file lists patterns to replace in the code block output before comparing it with the saved output. This helps when the output includes dynamic values like timestamps. If the output changes each time you run the doctest, add the relevant patterns to this file. | |
| The `doc/source/cluster/kubernetes/doc_sanitize.cfg` file lists patterns to replace in the code block output before comparing it with the saved output. This helps when the output includes dynamic values such as timestamps. If the output changes each time you run the doctest, add the relevant patterns to this file. |
Not sure what "If the output changes each time you run the doctest, add the relevant patterns to this file." indicates exactly. Are you encouraging folks to add configs here? The first goal should be idempotent code, correct? And then, for things that must differ with each run, fix it by ignoring that output.
| Limitations: You cannot use ``tab-set`` containing code blocks in Jupyter notebooks. | ||
| ``` | ||
|
|
||
| ## Running doctests locally |
There was a problem hiding this comment.
| ## Running doctests locally | |
| ## Run the doctests locally |
| python -m bash_kernel.install | ||
| ``` | ||
|
|
||
| ## Writing docs |
There was a problem hiding this comment.
| ## Writing docs | |
| ## Write doctests for KubeRay docs |
|
|
||
| The `doc/source/cluster/kubernetes/doc_sanitize.cfg` file lists patterns to replace in the code block output before comparing it with the saved output. This helps when the output includes dynamic values like timestamps. If the output changes each time you run the doctest, add the relevant patterns to this file. | ||
|
|
||
| ## Running doctests in CI |
There was a problem hiding this comment.
| ## Running doctests in CI | |
| ## Run doctests in CI |
|
|
||
| ## Running doctests in CI | ||
|
|
||
| Add the notebook filename to the `ci/k8s/run-kuberay-doc-tests.sh` file. |
There was a problem hiding this comment.
| Add the notebook filename to the `ci/k8s/run-kuberay-doc-tests.sh` file. | |
| To run the doctests for your notebook automatically as part of the Ray testing process, add the filename for your notebook to the `ci/k8s/run-kuberay-doc-tests.sh` file. |
Just wondering: do we need to add a sentence here about tests blocking the build of Docs and/or Ray release candidates? Who is responsible for fixing broken tests, etc?
I think this page is mostly targeting internal contributors, but this is part of the OSS docs so we should make sure we're very clear on expectations and outcomes (what is the bar testable notebook code needs to pass in order to be added to CI?)
Thanks @dstrodtman! @MortalHappiness would you mind chatting with @dstrodtman? |
|
This pull request has been automatically marked as stale because it has not had You can always ask for help on our discussion forum or Ray's public slack channel. If you'd like to keep this open, just leave any comment, and the stale label will be removed. |
github-actions
bot
added
the
stale
|
not stale |
github-actions
bot
removed
the
stale
|
we decided to remove doc tests. |
10 participants
Why are these changes needed?
As title.
doc link: https://anyscale-ray--51708.com.readthedocs.build/en/51708/ray-contribute/writing-kuberay-doctests.html
Related issue number
N/A
Checks
git commit -s) in this PR.scripts/format.shto lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/under thecorresponding
.rstfile.