-
Notifications
You must be signed in to change notification settings - Fork 285
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docs: Preserve output, clean eval notebooks #1590
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Hey @pbadhe, thanks for picking up this ticket. In addition to executing the notebooks to include input, we also need to alter the Line 199 in 50e765b
This command is causing the current pipeline failure: phoenix/.github/workflows/python-CI.yml Line 51 in 50e765b
|
@@ -197,7 +197,7 @@ fix = [ | |||
|
|||
[tool.hatch.envs.notebooks.scripts] | |||
clean = [ | |||
"jupyter nbconvert --ClearOutputPreprocessor.enabled=True --ClearMetadataPreprocessor.enabled=True --inplace **/*.ipynb **/**/*.ipynb", | |||
"jupyter nbconvert --ClearOutputPreprocessor.enabled=True --ClearMetadataPreprocessor.enabled=True --inplace **/*.ipynb **/internal/*.ipynb **/tracing/*.ipynb **/dolly-pythia-fine-tuned/*.ipynb", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had to do this as I couldn't find a negation(!) wildcard (for all *.ipynb except evals/*) that works in both windows and unix at the same time, apart from writing a new py script and then calling it in here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a shell command that works?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one does find . -name "*.ipynb" -not -path "./tutorials/evals/*" -exec jupyter nbconvert --ClearOutputPreprocessor.enabled=True --ClearMetadataPreprocessor.enabled=True --inplace {} \;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
We missed the pre-commit hook (which I think is sorta a problem that we have two checks). Will reopen the ticket |
Resolves #1559