Skip to content
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

FIX: Show traceback before raising ExecutionError #531

Merged

Conversation

paugier
Copy link
Contributor

@paugier paugier commented Jul 11, 2023

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
Copy link

welcome bot commented Jul 11, 2023

Thanks for submitting your first pull request! You are awesome! 🤗

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! 🎉

@agoose77
Copy link
Collaborator

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.

@agoose77 agoose77 self-requested a review August 23, 2023 09:35
@agoose77 agoose77 merged commit aae27bb into executablebooks:master Aug 23, 2023
@welcome
Copy link

welcome bot commented Aug 23, 2023

Congrats on your first merged pull request in this project! 🎉
congrats

Thank you for contributing, we are very proud of you! ❤️

@agoose77 agoose77 changed the title Show traceback before raising ExecutionError FIX: Show traceback before raising ExecutionError Sep 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants