-
Notifications
You must be signed in to change notification settings - Fork 947
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
Clarify how to fully clean widget state when updating versions #1866
Comments
Thanks - very good points! The "Clear widget state" menu option is for clearing the stored state in a notebook file, not for clearing the state on the page. You're right that (a) the language is confusing, and (b) it's frustrating that saving the notebook again saves all of the running state, which includes state originally from the document. I think we should make "clear widget state" clear the runtime state, as well as the document state - this sequence of steps always felt awkward to me. I usually add a step between 3 and 4, restart kernel, to make sure that I don't pull existing widget state from the kernel over to the notebook frontend. |
On the other hand, do you think we should have a menu option to just clear the runtime state? You can do this from python code with: from ipywidgets import Widget
Widget.close_all() ipywidgets/ipywidgets/widgets/widget.py Line 297 in a9c4069
|
Another option -- which I think it what I expected to happen at first -- would be for the "clear widget state" menu item to just clear the runtime state. "Save widget state" would be the only action that modifies the document. Then the widget-state operations would be:
In other words, the current "clear widget state [in document]" operation would be replaced by "clear [runtime] + save [runtime to document]", so there's no loss of functionality. Is that everything you want to do with widget states? Is restarting the kernel necessary? Is clearing cell outputs before clearing widget state necessary? |
I suppose the point about clearing cell outputs before clearing widget states is that it avoids left-over views with no model. But it's already possible to get into that state and it doesn't matter as the views are about to be overwritten in my case. |
you should never have views without a model - closing down a model gets rid of the views. I like your explanation above. Then
|
I think While we're on the topic, I feel the current
|
I don't think restarting a kernel closes all of the active widgets - I think they stick around in the frontend (sort of orphaned, but still providing models and views to the outputs). Perhaps any kernel restart should do a 'close all widgets' on the frontend, which will automatically clear them from the outputs. |
Another way to see this is to come back to the idea of a global notebook-level output area. At the moment, a widget front-end event coming from a model (i.e. a controller widget) cannot result in something being printed in a cell, because it does not come from a specific view and is not associated with any cell in particular. If there was a notion of non-cell-associated output area (toggable in the UI), we would still be able to get these events for debugging purposes. Also, this would have the benefit of letting up make the widget manager state part of the output, instead of the notebook-level metadata. Clear all output would then naturally remove the widget manager state from this notebook-level output. |
Another advantage of this, is that the |
Currently it's possible to use widgets without saving the state in the notebook (just by never clicking "save widget state"). If widget state is part of the output, you would save state to notebooks by default. That makes sense to me, but I wonder if there was a reason to not save state by default originally? |
At the very least, if you do a "Restart Kernel and Clear All Outputs" action, I can see no reason why it should not also close all widgets. This would basically give the user an avenue to avoid the browser refresh (step 4. in initial description), at low (no?) risk. |
I think saving widget manager state as part of the output (instead of in the notebook metadata) would make widgets behave as requested in #1632 too |
@ricklupton Given that the same widget can be used in several outputs, this doesn't seem feasible:
|
@vidartf I was referring to SilvainCorlay's comment about embedding widgets in notebook level (rather than cell-level outputs) -- which I think is currently just a suggestion but wouldn't suffer from those problems as I understand it. But maybe I misunderstood, I was just trying to link the discussion in the two issues! |
Adding to 7.2, with the idea that we can make some progress in clarifying the menu items here. As in the conversation above, I'm thinking the menu will now read (adding a close all item, and tweaking the wording):
|
Sounds good. Is it obvious that |
Ah, I was thinking it wouldn't automatically close all widgets. (Perhaps this is rehashing conversation above...) Should it? |
I read this ("could be done by ...") to mean it would close all widgets, is that what you meant? |
Right - I guess it isn't exactly equivalent, but perhaps that is too confusing anyway. |
In my opinion it would be simplest to understand if there was just one "save" command that modifies the widget state in the file on disk, which always makes the state on disk the same as the state you currently see, as I think I suggested above. So to clear state on disk you would "close all widgets" then "save widget state to notebook". The If you're worried about it needing two clicks then maybe follow the example of the But maybe that's just me. Either way I think the changes you suggested today would be a good improvement. |
Okay, you've convinced me. So here's the new proposal:
|
Tweaked wording:
Thoughts? Perhaps "Save Widgets to Notebook"? |
Sounds good! "Save Widgets to Notebook" is better, as you've done in #2012 |
I stumbled upon this issue while struggling with widgets that seem to be still alive in the front-end after a kernel restart. I have some custom "headless" widgets that wrap audio nodes (Web Audio API) or WebMIDI event listeners. When closing those widgets by calling
That would certainly be useful for my use case (actually I thought it was the case but apparently not? I'm using JupyterLab 4). Having a EDIT: #1866 (comment) would be nice as well. Or is there anything else I can do already to prevent such leaks in the front-end? |
I have some example notebooks in my custom widget repository that I want to update to use the latest version of the JS package whenever I release a new version, but I find it difficult to get it right on first try and often end up with old widget states still in my notebooks.
From experimenting I think the required steps are:
(this seems consistent with this comment)
Step 4 seems to be required -- perhaps some widget state persists in the client-side JS even after running "clear widget state". Is this intended? If so, can it be made clearer that this step is necessary, because running "clear widget state" sounds like it should be enough?
I wasn't sure what minimal example would help but I can try to put one together if needed.
The text was updated successfully, but these errors were encountered: