FIX: Show traceback before raising ExecutionError

paugier commented 1 year ago

When

nb_execution_raise_on_error = True
nb_execution_show_tb = True

the traceback is not showed, which is very inconvenient when it happens during a CI job!

I guess it is only an issue of incompatibility between these two options, which should be fixed by this swap.

Does it make sense?

welcome[bot] commented 1 year ago

Thanks for submitting your first pull request! You are awesome! :hugs:
If you haven't done so already, check out EBP's Code of Conduct and our Contributing Guide, as this will greatly help the review process.
Welcome to the EBP community! :tada:

agoose77 commented 1 year ago

This PR looks reasonable. For context, I don't think this option is that useful.

If you're only using myst-nb, one can pass the -T or -v arguments to sphinx-build, which show tracebacks in stderr, if nb_execution_raise_on_error is set to True.
If you're using jupyter-book, the same applies (enable verbose output -v), but it's more tricky to set this option as it's not passed through from _config.yml. You can patch the Sphinx config with
```
sphinx:
recursive_update: true
config:
  nb_execution_raise_on_error: true
```

Despite this, I don't see any harm in this PR, and it might prove useful.

welcome[bot] commented 1 year ago

Congrats on your first merged pull request in this project! :tada:
Thank you for contributing, we are very proud of you! :heart:

executablebooks / MyST-NB

FIX: Show traceback before raising ExecutionError #531