Do not suppress exceptions in Output widget context #3417
Conversation
|
|
wookayin
changed the title
| @@ -60,6 +67,8 @@ def func(): | |||
| msg_id = Unicode('', help="Parent message id of messages to capture").tag(sync=True) | |||
| outputs = TypedTuple(trait=Dict(), help="The output messages synced from the frontend.").tag(sync=True) | |||
|
|
|||
| catch_exception = Bool(False, help="Whether to catch and suppress all exceptions.").tag(sync=True) | |||
There was a problem hiding this comment.
This probably doesn't need to be synced (?). Making it backwards compatible by keeping the current default would also be a clear preference on my end.
There was a problem hiding this comment.
So this could be an attribute rather than a trait? In such cases should I add a custom __init__ constructor? Or would just sync=False (or not having it) suffice?
There was a problem hiding this comment.
Just not including the .tag(sync=True) should be sufficient for keeping it as a type-checked trait that is not synced to the front-end.
| if kernel: | ||
| if self.catch_exception: | ||
| return True |
There was a problem hiding this comment.
Nit-pick :)
| if kernel: | |
| if self.catch_exception: | |
| return True | |
| if kernel and self.catch_exception: | |
| return True |
|
Interestingly, in JupyterLite it seems that an output widget does not capture exceptions, even though it does in ipykernel. |
|
From @vidartf:
I agree with Vidar. A very common case for output widgets is to capture output that would not otherwise be visible (like widget callbacks), and capturing and displaying exceptions is very valuable for seeing why callbacks did not work. Also, I think if there is a question, preserving backwards compatibility should have a high priority (there is a lot of widgets code out there that people don't want to change if they can help it) |
|
@vidartf, any possibility this can be merged in 8.0? I can make requested changes quickly. It's kind of a breaking change, so I feel it would be included in the major version. UPDATE: never mind, I missed that 8.0 has been already out. Looking forward to be merged in the next release. Should we also add warnings for changing default behaviors in future versions, say 9.x? |
…3208) Output widgets, when used as context managers in a with statement and in IPython, were suppressing exceptions (whose stacktrace would be printed to the output widget). This change allows Output widgets to suppress an exception when `catch_exception` is set to False, which would make the control flow of the code transparent and same as if output widgets were not used. For backward compatibility reasons, the default behavior is not yet changed at the moment of ipywidgets 8.0, i.e., the context manager of Output widgets will consume and suppress any exceptions thrown. However, at later versions the default behavior may change.
wookayin
force-pushed
the
output-exception
branch
from
8b086a6 to
f65dae6
Compare
Output widgets, when used as context managers in a with statement and in IPython, were suppressing exceptions (whose stacktrace would be printed to the output widget).
This change makes Output widgets not suppress an exception, which will make the control flow of the code transparent and same as if output widgets were not used.
For compatibility reasons, the previous behavior can be achieved by setting
capture_exceptionattribute to True, in which case the context manager will suppress any exceptions thrown (if any).Fixes #3208
TODO: