ipython-contrib / jupyter_contrib_nbextensions

A collection of various notebook extensions for Jupyter
http://jupyter-contrib-nbextensions.readthedocs.io/en/latest
Other
5.22k stars 806 forks source link

handling `notebook_loaded.Notebook` events correctly #885

Open jcb91 opened 7 years ago

jcb91 commented 7 years ago

As discussed in #879, sometimes (particularly with big notebooks, slow connections, etc), nbextensions get loaded before the notebook has finished loading. In other cases, the notebook may have fully loaded before the nbextension gets loaded, so the nbextension will miss the notebook_loaded.Notebook event. Finally, the event is fired again as part of loading checkpoints, so it's not necessarily a one-time-only event! Nbextension code should be able to deal with any of these three scenarios.

The best way to approach this and still get any initialization which needs to be applied to the whole notebook performed correctly seems to be:

  1. make sure any whole-notebook initialization function can be called multiple times without causing problems like duplicating html elements, etc.
  2. bind the initialization function to the notebook_loaded.Notebook event, so that it gets re-initialized correctly whenever the notebook is (re)loaded
  3. As part of the load_ipython_extension function used to load the nbextension, check if Jupyter.notebook._fully_loaded === true, which indicates we missed the first notebook_loaded.Notebook event. If this is true, call the initialization function(s) mentioned in 1 directly

I've had a quick skim through our nbextensions looking for ones which may need their loading behaviour altered, and come up with the following lists:

The following need updates to handle loading correctly:

These I'm uncertain about:

These seem to run at least some code without load_ipython_extension being called, which I don't think is supposed to happen, so perhaps need a bit of reworking:

These haven't even been updated to notebook 4.x, so should either be updated or removed:

These already use the correct approach:

And these don't need notebook loaded to initialize:

odedbd commented 7 years ago

I tested hide_input and I can confirm that it does not handle the refresh properly on a large notebooks.

kukanya commented 7 years ago

scroll_down -ok but needs update to handler for new '.output' instances

Could you elaborate a little bit more on that? I don't exactly understand what I need to fix.

jcb91 commented 7 years ago

So, I may have misunderstood, but I think the code which attaches the resize handler (in scroll_down/main.js#L47-L54 attaches it to any elements with class output that exist when the call is made (i.e. when the nbextension loads), but not to any cells created afterwards (which may, as a result of the conditions discussed above, also include some or all of the cells in the notebook still being loaded).

I'd suggest that the solution is either to catch create_cell.Cell events to attach the handler to new cells as well, or to attach the handler to a parent element like #notebook-container with a selector (see jquery API docs for details of this last).

Does that make sense?

juhasch commented 7 years ago

Let's remove the stale history and read-only extensions. For read-only using the freeze extension is better and has more functionality.

Btw. in the notebook 5.0 there will be is_deletable() and is_editable() functions for each cell and the corresponding metadata metadata.deletable and metadata.editable. See https://github.com/jupyter/notebook/blob/master/notebook/static/notebook/js/cell.js

soamaven commented 7 years ago

Could this be why my hidden input cells are not hidden on reload (have a large notebook)?

jcb91 commented 7 years ago

@soamaven if you're still using an older version, perhaps. However, also possibel is that the large notebook causes the nbextension loading to take too long, and so it gets abandoned by requirejs, as in #822. Do you see any log messages in the javascript console?

soamaven commented 7 years ago

Using the latest version on conda, jupyter-contrib-nbextensions==0.2.8

I don't see anything obvious, maybe the script error for 'typo/typo' near the end?

Paste is Here

jcb91 commented 7 years ago

Hmm, yeah nothing obvious in the console logs, certainly. I think we've already fixed hide_input to handle the events correctly, but perhaps there's some interaction with another extension... do you still see the same behaviour with all other nbextensions disabled?

soamaven commented 7 years ago

Hey thanks that worked! Most of the extensions play nice, looks like jupyter-js-widgets/extension was the problem

jcb91 commented 7 years ago

oh, weird, I've no idea why that should be interacting there :confused: