-
Notifications
You must be signed in to change notification settings - Fork 55
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
Optionally write out executed notebook in jupyter-execute #307
Optionally write out executed notebook in jupyter-execute #307
Conversation
Fixes jupyter#306. Now actually write out the executed notebook to the same path as the input.
This commit adds two options to optionally save the executed notebook. * `--inplace`: Save the executed notebook to the input notebook path * `--output`: Save the executed notebook to `output`. This option can take a pattern like `{notebook_name}-new`, where `notebook_name` is the name of the input notebook without extension `.ipynb`. Also, the output location is always relative to the input notebook location.
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.
LGTM, thanks!
The failure seems relevant, though I'm not sure what is triggering it about the line: nbclient/cli.py:200: in run_notebook
with input_path.open() as f:
...
E DeprecationWarning: pathlib.Path.__enter__() is deprecated and scheduled for removal in Python 3.13; Path objects as a context manager is a no-op |
I think the error is due to how I mocked things. Working on it... Yup, that was due to mocking |
Head branch was pushed to by a user without write access
No clue why CI is/was failing for macos 3.10. I merged in changes from main. Hopefully that fixes it... |
The macos tests have been flaky for some time now. |
Thanks again! |
My pleasure! |
name = notebook_path.replace(".ipynb", "") | ||
input_path = Path(notebook_path).with_suffix(".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.
@wpk-nist-gov Pardon my potential ignorance, but doesn't this change the input behavior?
The code was initially stripping the suffix then adding it, but this change adds the suffix indiscriminately. Seems if I pass a path to a notebook, the ipynb suffix will be duplicated.
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.
with_suffix
respects if the path already has the extension. For example:
>>> from pathlib import Path
# with suffix already there, don't replace
>>> Path("a/b/c.ipynb").with_suffix(".ipynb")
PosixPath('a/b/c.ipynb')
# without suffix, add it
>>> Path("a/b/c").with_suffix(".ipynb")
PosixPath('a/b/c.ipynb')
The previous behavior was to replace, then add the suffix:
name = notebook_path.replace(".ipynb", "")
input_path = f"{name}.ipynb"
These should be mostly equivalent...
Fixes #306.
Now actually write out the executed notebook to the same path as the input.