do not execute notebooks - #93
Conversation
WalkthroughThe Sphinx documentation configuration was updated to disable automatic execution of Jupyter notebooks during builds by changing the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/conf.py (1)
319-321: Gate notebook execution via environment variable instead of hard-coding.Hard-coding
nbsphinx_execute = "never"stops all local builds from executing notebooks as well. A more flexible pattern is to disable execution only on RTD (or CI) and keep the default ("auto"/"always") for local docs so developers still catch notebook issues early.-# Always skip execution (RTD test build) -nbsphinx_execute = "never" +# Skip execution on RTD/CI, run notebooks locally +if os.environ.get("READTHEDOCS") == "True": + nbsphinx_execute = "never" +else: + # Use "auto" to execute notebooks only if their output is missing/outdated + nbsphinx_execute = "auto"This keeps local feedback loops fast while still meeting the PR objective of avoiding execution in the RTD builders.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/conf.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build
- GitHub Check: test (windows-latest, 3.12)
- GitHub Check: test (windows-latest, 3.12)
|
📖 Documentation Preview The documentation for this PR has been built and is available at: This preview will be updated automatically when you push new commits to this PR. Preview built from commit: |
|
📖 Documentation Preview The documentation for this PR has been built and is available at: This preview will be updated automatically when you push new commits to this PR. Preview built from commit: |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
.github/workflows/execute_notebooks.yml (2)
29-33: Bumpactions/setup-pythonto the latest major
v5is now the current major and brings cache-key fixes and Python 3.13 preview images. Staying onv4risks deprecation warnings soon.- uses: actions/setup-python@v4 + uses: actions/setup-python@v5
35-46: Make APT non-interactive and explicitWithout
-y,apt installcan hang if a prompt is ever shown. Usingapt-getis also the conventional choice in CI.- sudo apt update - sudo apt install pandoc + sudo apt-get update -y + sudo apt-get install -y pandoc
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/execute_notebooks.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/execute_notebooks.yml
30-30: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
81-81: property "verify-changed-files" is not defined in object type {}
(expression)
86-86: property "verify-changed-files" is not defined in object type {}
(expression)
🪛 YAMLlint (1.37.1)
.github/workflows/execute_notebooks.yml
[error] 21-21: trailing spaces
(trailing-spaces)
[warning] 23-23: wrong indentation: expected 6 but found 4
(indentation)
[error] 40-40: trailing spaces
(trailing-spaces)
[error] 61-61: trailing spaces
(trailing-spaces)
[error] 71-71: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build
- GitHub Check: test (windows-latest, 3.12)
- GitHub Check: test (windows-latest, 3.12)
| if: steps.verify-changed-files.outputs.changed == 'true' && github.event_name != 'pull_request' | ||
| run: | | ||
| git push origin ${{ github.ref_name }} | ||
|
|
||
| - name: Summary | ||
| run: | | ||
| if [ "${{ steps.verify-changed-files.outputs.changed }}" == "true" ]; then | ||
| echo "✅ Notebooks executed successfully and changes committed" |
There was a problem hiding this comment.
Undefined verify-changed-files output
The if: expressions reference steps.verify-changed-files, but no step with that ID exists, so the expression resolves to null and the workflow will fail during JSON-pointer evaluation.
Either add the missing step (e.g. tj-actions/verify-changed-files) or drop the conditional:
- if: steps.verify-changed-files.outputs.changed == 'true' && github.event_name != 'pull_request'
+ if: github.event_name != 'pull_request' && git diff --quiet HEAD || echo "changes"Adjust the summary step similarly.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 actionlint (1.7.7)
81-81: property "verify-changed-files" is not defined in object type {}
(expression)
86-86: property "verify-changed-files" is not defined in object type {}
(expression)
🤖 Prompt for AI Agents
In .github/workflows/execute_notebooks.yml around lines 81 to 88, the workflow
references outputs from a step with ID 'verify-changed-files' which does not
exist, causing the workflow to fail. To fix this, either add the missing step
that sets this output (such as using the 'tj-actions/verify-changed-files'
action with the ID 'verify-changed-files') before these lines, or remove the
conditional checks that reference 'steps.verify-changed-files.outputs.changed'
in both the push and summary steps to avoid using undefined outputs.
|
📖 Documentation Preview The documentation for this PR has been built and is available at: This preview will be updated automatically when you push new commits to this PR. Preview built from commit: |
|
📖 Documentation Preview The documentation for this PR has been built and is available at: This preview will be updated automatically when you push new commits to this PR. Preview built from commit: |
|
📖 Documentation Preview The documentation for this PR has been built and is available at: This preview will be updated automatically when you push new commits to this PR. Preview built from commit: |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/execute_notebooks.yml (1)
81-88: Undefinedverify-changed-filesreference will make the workflow fail
if: steps.verify-changed-files.outputs.changed …is still present, but no step with the IDverify-changed-filesis defined, so the expression resolves to null and GitHub Actions aborts before this step even runs.
This exact problem was raised in the previous review and has not been addressed.+ # Detect whether any notebook files changed after execution + - name: Verify changed files + id: verify-changed-files + uses: tj-actions/verify-changed-files@v18 + with: + files: | + docs/**/*.ipynbThen keep the existing Push changes / Summary
if:expressions.
🧹 Nitpick comments (2)
.github/workflows/execute_notebooks.yml (2)
39-40: Make APT installs non-interactive
apt installcan prompt for confirmation and hang a CI job. Pass-y(or useapt-get) to ensure unattended execution.- sudo apt update - sudo apt install pandoc + sudo apt-get update -y + sudo apt-get install -y pandoc
57-58: Only notebooks underdocs/Data Formatting/are discoveredIf additional notebooks exist directly under
docs/(besides Getting Started), they won’t be executed. A broader search keeps the list maintenance-free.- find "docs/Data Formatting/" -name "*.ipynb" > notebooks_to_execute.txt - echo "docs/Getting Started.ipynb" >> notebooks_to_execute.txt + find docs -maxdepth 2 -name "*.ipynb" > notebooks_to_execute.txt
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/execute_notebooks.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/execute_notebooks.yml
31-31: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
82-82: property "verify-changed-files" is not defined in object type {}
(expression)
87-87: property "verify-changed-files" is not defined in object type {}
(expression)
🪛 YAMLlint (1.37.1)
.github/workflows/execute_notebooks.yml
[error] 22-22: trailing spaces
(trailing-spaces)
[error] 41-41: trailing spaces
(trailing-spaces)
[error] 62-62: trailing spaces
(trailing-spaces)
[error] 72-72: trailing spaces
(trailing-spaces)
🔇 Additional comments (1)
.github/workflows/execute_notebooks.yml (1)
30-34: Consider upgrading toactions/setup-python@v5
v4is flagged by actionlint as too old for the current runner image. Updating avoids sporadic cache issues and future deprecation warnings.- - name: Set up Python - uses: actions/setup-python@v4 + - name: Set up Python + uses: actions/setup-python@v5
| - name: Configure git | ||
| run: | | ||
| git config --local user.email "action@github.com" | ||
| git config --local user.name "GitHub Action" | ||
|
|
||
| - name: Find and execute notebooks | ||
| run: | |
There was a problem hiding this comment.
🛠️ Refactor suggestion
No step commits the modified notebooks
You configure Git but never stage or commit the executed notebooks, so even with a correct if: guard nothing will be pushed.
- name: Configure git
run: |
git config --local user.email "action@github.com"
git config --local user.name "GitHub Action"
+ - name: Commit notebook changes
+ run: |
+ if ! git diff --quiet; then
+ git add docs/**/*.ipynb
+ git commit -m "ci: update executed notebook outputs [skip ci]"
+ fiCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In .github/workflows/execute_notebooks.yml around lines 49 to 55, after
configuring Git user details, add steps to stage the modified notebooks using
'git add' and commit them with an appropriate commit message. This ensures that
the executed notebooks are committed and can be pushed later in the workflow.
1 participant
See if RTD builds work if do not execute notebooks
Summary by CodeRabbit