-
Notifications
You must be signed in to change notification settings - Fork 507
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
Add cell error instruction configurable #471
Add cell error instruction configurable #471
Conversation
LGTM. |
Good idea @aschlaep. I think however, the trait should go into I'm happy to make those changes for you if you want. |
@aschlaep could you push the changes that you just showed me so that I can try out the PR and the issue with default values. |
I added the default message in execute.py because the default value of the traitlet in configuration.py wasn't getting passed through for some reason... |
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.
Yes, this makes much more sense, and much cleaner. I left some suggestions to make it even simpler.
voila/execute.py
Outdated
instruction = 'Please run Voila with --debug to see the error message.' | ||
if 'VoilaConfiguration' in self.config and 'cell_error_instruction' in self.config['VoilaConfiguration']: | ||
instruction = self.config['VoilaConfiguration']['cell_error_instruction'] |
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 can be removed.
voila/execute.py
Outdated
if 'VoilaConfiguration' in self.config and 'cell_error_instruction' in self.config['VoilaConfiguration']: | ||
instruction = self.config['VoilaConfiguration']['cell_error_instruction'] | ||
|
||
error_message = 'There was an error when executing cell [{}]. {}'.format(cell['execution_count'], instruction) |
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.
error_message = 'There was an error when executing cell [{}]. {}'.format(cell['execution_count'], instruction) | |
error_message = 'There was an error when executing cell [{}]. {}'.format(cell['execution_count'], this.cell_error_instruction) |
… debug-message-config
Thanks @aschlaep ! |
* add cell error instruction configurable in VoilaExecutePreprocessor
* add cell error instruction configurable in VoilaExecutePreprocessor
This adds a VoilaConfiguration option to allow customization of the debug instruction when there's a cell error. It may not always make sense to say "Please run Voila with --debug ...", for example when voila is run as a notebook/server extension. This'll allow people to supply their own instructions to the user for how to remediate potential bugs.
If anybody has any suggestions for a better name than
cell_error_instruction
, please let me know!