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

feat(python): use returncode for python exec #80

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

ianhi
Copy link
Contributor

@ianhi ianhi commented Feb 28, 2025

Formats and displays any exceptions when returncode is not 0.

This is helpful if you want to demonstrate code that you expect to raise an error. Doubly so when you are running wiht mkdocs build --strict

Example rendering in mkdocs-material:

```python exec="on" source="above" returncode="1" result="python"
print("Hello Markdown!")
raise ValueError
```
image

Formats and displays any execeptions when `returncode` is not 0.
@ianhi
Copy link
Contributor Author

ianhi commented Mar 1, 2025

May also make sense to use redirect_stdout and redirect_stderr (https://docs.python.org/3/library/contextlib.html#contextlib.redirect_stdout) instead of patching the print function. In case there are expected outputs that never get printed

Copy link
Owner

@pawamoy pawamoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @ianhi, thanks a lot for the PR! I like it. Could you update the docs to add a section similar to the Shell one, into the Python page? And update the main Usage page to reference both 🙂

@@ -77,7 +77,10 @@ def _run_python(
frame._lines = _code_blocks[frame.filename][frame.lineno - 1] # type: ignore[attr-defined,operator]
else:
frame._line = _code_blocks[frame.filename][frame.lineno - 1] # type: ignore[attr-defined,operator]
raise ExecutionError(code_block("python", "".join(trace.format()), **extra)) from error
if returncode not in [None, 0]:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if returncode not in [None, 0]:
if returncode not in (None, 0):

@ianhi
Copy link
Contributor Author

ianhi commented Mar 2, 2025

After sleeping on it I've realized it might be nice to be able to specify the exception type, rather than returncode. How do you feel about adding a new field like exception="ValueError" then making sure to match that, and not having returncode do anything for python?

@ianhi
Copy link
Contributor Author

ianhi commented Mar 2, 2025

Ok, latest push implements that idea. I think I have one more section of docs to update but curious if you prefer this approach. Switching to exception matching for python/pycon (but not pyodide) and returncode just for shell

@pawamoy
Copy link
Owner

pawamoy commented Mar 4, 2025

I have thought about it and I think I'd prefer we stay with returncode for now, because asserting exception types is under-specified here. What if users write exception="some_package.SomeException"? We'd have to actually import the object dynamically to compare it against the caught exception (as asserting the type as you've done it here, using string in message, wouldn't work, and is not robust enough IMO). Users could also want to use the local scope, i.e. exception="SomeException" because SomeException was imported in the executed code block. Also, they might want to assert more than the type, for example the exit code in SystemExit, or the actual message of the exception, etc.. This brings a lot of complexity that we'd have to discuss in an issue first 🙂

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