-
Notifications
You must be signed in to change notification settings - Fork 431
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
Tag regex #403
Conversation
I've tested this on my machine and find it very useful. Would prefer if someone else fixes the unit tests. |
fixed unit tests and added basic CLI parsing tests. Would be nice if someone added an actual test to verify correct cell tag parsing. |
I was trying and testing this PR at my local, and it worked out great. However, when using cli it is important to note that quoting and escaping mechanism is different for windows batch and linux shell. windows: papermill notebooks\input_notebook.ipynb notebooks\output_notebook.ipynb -t ^^(import^|data)$ shell: papermill notebooks/input_notebook.ipynb notebooks/output_notebook.ipynb -t '^(import|data)$' |
So I don't like pushing back on contributions, but I might pause this PR here and suggest this shouldn't be in papermill core. It adds a lot of complexity to the AST execution pattern and makes it more difficult to reason about how a notebook will be executed. Papermill is meant to be a very simple tool with few, opinionated options around templated execution of notebooks. This would bend that simplicity. Furthermore, I think there's already a decent execution pattern for removing cells with particular tags via nbconvert. Specifically the TagRemovePreprocessor allows for one to make a new notebook file that has the tags prunned out. This helps preserve a functional execution pattern where you have the intermediate pruned object pass along to the simpler execution tool in papermill.
Does this sound reasonable and understandable? |
Hi @casperdcl. Thanks for using papermill as well as taking the time to explore new uses and to contribute back updates for tqdm. As @MSeal commented, this PR's use case today falls outside of the scope of papermill's lightweight design. Papermill's current design goal is to be a simple tool to add parameters to a notebook and automate a notebook's execution with the parameters. Your proposed PR goes beyond that goal by adding logic to extend the use of tags beyond parameters for variables within a code cell. This PR introduces the use of additional tags (such as debug) to decorate a notebook cell and control which cells are executed based on the decorated tag. Right now, this is beyond the scope of the core papermill project. It's an interesting use case to use tags as decorators to determine which cells are executed. I'm going to close this PR as being beyond the scope of the current project core. I'm also going to label this PR as |
examples