DRAFT: Single widget manager per kernel #3920
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…dentify closed widgets. Added a new class `Children` for the children trait of Box optimised for checking widgets and quietly dropping widgets that are closed, and objects that aren't widgets. Widgets in Box.children are also removed when the widget is closed. Added tests test_gc_box & test_gc_box_advanced
…pr_mimebundle_ is set to None when closed, this reduces the risk of name clash with subclasses. Changed Children to allow any object that has the method `_repr_mimebundle_`. Moved box related tests to test_widget_box. Added `close` method to Box to ensure discarded children are un-observed.
…rgument `observe_children`. Updated TestBox.
- change evaluation order to check for a widget first (most common object). - raise NotImplementedError for objects with _repr_mimebundle_ that aren't widgets.
Fix 'Exception' for a Box that has been provided a tooltip.
Remove observe_children from 'Box'
added 8 commits
May 5, 2024 12:05
Removed _model_id and revised __rer__ for a closed widget. Added validate_mode for Box.
…idget available for gc).
…dgetManager a singleton on a per kernel basis. Other fixes to make widgets more accessible.
|
|
|
Thanks for opening a PR for this! Was this closed on purpose? |
|
Yes and no. If there is still the possibility of a per-kernel widget manager I can re-open or make a new PR with updates. |
|
I believe we're still open to refactor the widget manager logic to be more "general" and less tight to the Notebook (or the console). #3004 was merged as-is because it's already an improvement of the current state, but Jason's comments in the PR are still relevant. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is exploring the possibility of a per-kernel widget manager as suggested by @jasongrout in the comments of the recently merged PR: #3004.
This PR follows on from a recent PR #3893, but could be easily re-based independently.
In it's current form, it can render the same widget in notebook[s] and consoles connected to the same kernel. The only requirement being that the KernelWidgetManager is registered to (instantiated with) the kernel.
How it works
KernelWidgetManagerwas modified to create only one instance per kernel.id.WidgetManagerwas changed to be an interface for notebooks that swaps the notebook (panel) rendermime factory to use the KernelWidgetManager corresponding to the new kernel as the kernel is changed.Other changes
WidgetRendererwas modified to search for the widget manager of a model_id if the widgetManager wasn't specified.