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

freeze state of cells is not persisted when reloading notebook or refreshing page (browser refresh) #879

Closed
odedbd opened this issue Feb 9, 2017 · 29 comments · Fixed by #884

Comments

@odedbd
Copy link

odedbd commented Feb 9, 2017

The freeze extension answers a very real use case for me, but unfortunately it doesn't seem to persist the cell's status after closing the notebook and reopening it, or even when simply refreshing the page. This seems strange as the documentation indicates that the state is saved in the cell's metadata and should be preserved.

Moreover, I used the Edit cell metadata option to verify that the cell does indeed have in its metadata the settings that it should be frozen, namely-

 {
  "scrolled": true,
  "deletable": true,
  "collapsed": false,
  "hide_input": false,
  "editable": true,
  "run_control": {
    "frozen": true,
    "read_only": true
  },
  "trusted": true
}

Yet it is not colored as frozen and indeed I can modify its contents after refreshing the page. Setting it as frozen again will work until I refresh the page again.

@jfbercher
Copy link
Member

What other extensions do you use?
Can you disable all the other extensions to see if there is not a conflict with another one?

@odedbd
Copy link
Author

odedbd commented Feb 9, 2017

Good idea. I have a quite a few extensions enabled, but this is on my work computer so I will only be able to check the list and test for a conflict on Sunday (weekend where I live is Friday-Saturday).

Thanks!

@jcb91
Copy link
Member

jcb91 commented Feb 10, 2017

Pinging @kukanya as freeze's author. This may also be simply related to the notebook being saved or not. In order for metadata changes to be correctly interpreted as changes to the notebook, the notebook's state should be designated 'dirty' (see notebook/static/notebook/js/notebook.js#L385-L396). If you freeze a cell, then directly refresh the page, the chances are that the metadata changes will not have been saved, but the notebook isn't set as dirty, so won't warn you about leaving the page. As a result, on reloading, the cell won't be frozen anymore. You can check whether this is the cause of your issue just by making sure to save the notebook before refreshing the page. Actually, it could be worth setting the notebook as dirty on freezing/unfreezing cells to avoid this kind of problem, regardless of whether it's actually causing this particular issue.

@kukanya
Copy link
Contributor

kukanya commented Feb 10, 2017

@jcb91, thank you for making me aware.

I'll try to reproduce the issue this weekend and work on a fix if it's reproducible for me.

And yes, the extension is not designed to apply changes you do directly to the metadata. I guess I'll work on that if I have time, too.

Thanks for positive feedback too. Good to know people actually make use of Freeze.

@jcb91
Copy link
Member

jcb91 commented Feb 10, 2017

Yes, most of the nbextensions we have that modify metadata don't bother to set the notebook dirty state, largely because it rarely matters (usually, you do something else as well which does set the notebook state to dirty before it matters).

@odedbd
Copy link
Author

odedbd commented Feb 11, 2017

I made sure to save the notebook before refreshing, I should have mentioned it in my post. I'll check my on my work computer tomorrow which other extensions I have installed and report back.

@kukanya - freeze is really great for my use case. I often run long-running computations that would be a real pain to reproduce just because it would take a lot of time (>1day). Being able to protect from accidentally executing the cell (which would clear its output) is really great. I do have a routine backup procedure in place that I could use if I mess up, but being able to prevent the problem is much better than being able to recover from it.

@jcb91
Copy link
Member

jcb91 commented Feb 11, 2017

I made sure to save the notebook before refreshing

Ok, so it's not the set_dirty thing then!

@odedbd
Copy link
Author

odedbd commented Feb 12, 2017

Just to double check I have decided to redo the installation of jupyter_contrib_nbextensions, this time using pip -I. Then I disabled my browser cache and tested again. Everything works like a charm now.

The freeze state is preserved both when refreshing the notebook and when closing and restarting it. I have no idea what was wrong before, but everything is now working.

I'll close the issue now, thanks for your help!

@odedbd odedbd closed this as completed Feb 12, 2017
@odedbd
Copy link
Author

odedbd commented Feb 12, 2017

Looks like I closed this too soon. I am not sure what changed but this is now not working again. I tried disabling all contrib extensions other than freeze. I currently have no extensions enabled that I can see in the configurator.

Strange thing is that the persistence no longer works also in the notebook on which I tested it before where it worked.

@odedbd odedbd reopened this Feb 12, 2017
@odedbd
Copy link
Author

odedbd commented Feb 12, 2017

Something even stranger now - even when I freeze a cell or make it read only I can now still edit its source. This was not the case before so it is really strange. The cell's background color does change, but other than that there is no effect.

@juhasch
Copy link
Member

juhasch commented Feb 12, 2017

Can you try a private browser window ?
This should make sure that there is no stale Javascript.
Also, can you check again if the metadata of the cell shows the frozen state ?

@odedbd
Copy link
Author

odedbd commented Feb 12, 2017

The same thing happens in a private browser window. The frozen cell has the following metadata, please note that some of the fields are from extensions that were enabled before but are no longer enabled.

{
  "ExecuteTime": {
    "start_time": "2017-02-12T11:49:39.381000",
    "end_time": "2017-02-12T11:49:42.703000"
  },
  "run_control": {
    "frozen": true,
    "read_only": true
  },
  "editable": true,
  "deletable": true,
  "collapsed": false,
  "trusted": true
}

@odedbd
Copy link
Author

odedbd commented Feb 12, 2017

Not sure if it helps or not, but here is what I see in the chrome dev console when loading the notebook-

Build Database.ipynb:121 loaded custom.js
default.js:48 Default extension for cell metadata editing loaded.
rawcell.js:82 Raw Cell Format toolbar preset loaded.
slideshow.js:43 Slideshow extension for metadata editing loaded.
menubar.js:240 actions jupyter-notebook:find-and-replace does not exist, still binding it in case it will be defined later...
MenuBar.bind_events @ menubar.js:240
MenuBar @ menubar.js:55
(anonymous) @ main.js:102
execCb @ require.js?v=6da8be3…:1690
check @ require.js?v=6da8be3…:865
(anonymous) @ require.js?v=6da8be3…:1140
(anonymous) @ require.js?v=6da8be3…:131
(anonymous) @ require.js?v=6da8be3…:1190
each @ require.js?v=6da8be3…:56
emit @ require.js?v=6da8be3…:1189
check @ require.js?v=6da8be3…:940
(anonymous) @ require.js?v=6da8be3…:1140
(anonymous) @ require.js?v=6da8be3…:131
(anonymous) @ require.js?v=6da8be3…:1190
each @ require.js?v=6da8be3…:56
emit @ require.js?v=6da8be3…:1189
check @ require.js?v=6da8be3…:940
enable @ require.js?v=6da8be3…:1177
init @ require.js?v=6da8be3…:783
callGetModule @ require.js?v=6da8be3…:1204
completeLoad @ require.js?v=6da8be3…:1604
onScriptLoad @ require.js?v=6da8be3…:1711
utils.js:59 load_extensions ["jupyter-js-widgets/extension", "nbextensions_configurator/config_menu/main", "freeze/main", "contrib_nbextensions_help_item/main"]
utils.js:36 Loading extension: nbextensions_configurator/config_menu/main
utils.js:36 Loading extension: contrib_nbextensions_help_item/main
utils.js:36 Loading extension: freeze/main
main.js?v=20170212115548:34 [Freeze] patching CodeCell.prototype.execute
main.js?v=20170212115548:20 [Freeze] patching MarkdownCell.prototype.unrender
session.js:54 Session: kernel_created (27ba3bd9-b6b1-4520-abe8-9d6c0bd49421)
kernel.js:456 Starting WebSockets: ws://localhost:8888/api/kernels/d86cae47-3cdc-4a2c-9c6e-548fff58a764
kernel.js:101 Kernel: kernel_connected (d86cae47-3cdc-4a2c-9c6e-548fff58a764)
utils.js:36 Loading extension: jupyter-js-widgets/extension
kernel.js:101 Kernel: kernel_ready (d86cae47-3cdc-4a2c-9c6e-548fff58a764)
extension.js:88 loaded widgets
manager-base.js:195 Widget backend and frontend versions are compatible

@jcb91
Copy link
Member

jcb91 commented Feb 12, 2017

Not sure if it helps or not, but here is what I see in the chrome dev console when loading the notebook-

Definitely helpful in the sense that it show us that nothing obvious is going wrong in the javascript (thanks for this!), but I'm afraid it doesn't (to me) suggest any obvious cause for the issue. Given the intermittent nature of the problem, I'd guess there maybe something going on with timing of patches somewhere, but I'm not sure.

@odedbd
Copy link
Author

odedbd commented Feb 12, 2017

I think I may have a bigger problem than just something specific to freeze. I just test the Execute Time extension, and although it works fine when executing a cell (the queue and execution times are shown nicely), after saving the notebook and restarting it the execution times are not shown. I verified that they appear in the cell metadata, and they are. It looks as though things are generally broken on my install and I am not sure as to why. I tried completely uninstalling Jupyter and all related packages, deleting the .jupyter folder and then reinstalling everything and it didn't change anything.

@jcb91
Copy link
Member

jcb91 commented Feb 12, 2017

I verified that they appear in the cell metadata, and they are. It looks as though things are generally broken on my install and I am not sure as to why.
I tried completely uninstalling Jupyter and all related packages, deleting the .jupyter folder and then reinstalling everything and it didn't change anything.

Hmm, ok, this sounds again like it may be something timing-related. The ExecuteTime nbextension doesn't really make any effort to check that the notebook's finished loading before trying to add existing timing info to cells, so if it runs before the notebook finishes loading, any cells which load afterwards will be missing their timing information. I can try to patch that. Do you have any interesting log messages from the server side of things? Also, what notebook version are you using? We haven't tested everything with 5.0 yet, so there may be issues to catch with that.

@odedbd
Copy link
Author

odedbd commented Feb 12, 2017

notebook version from pip freeze is notebook==4.4.1

My notebooks are pretty large, so they take a lot of time to load, so timing issues sound like a possible reason.

@juhasch
Copy link
Member

juhasch commented Feb 12, 2017

I agree, it could be related to large notebooks and the extensions not handling this properly, as @jcb91 suggests.
Did you experience this problem with small notebooks, too?

@odedbd
Copy link
Author

odedbd commented Feb 12, 2017

I created a single cell notebook where the cell just has a simple print statement, and found the following-

  1. the execution times extension works fine, the execution time is shown upon load without a hitch
  2. the freeze status persistence works fine upon reload, so this was indeed a timing issue
  3. the freeze status works as far as preventing cell execution.
  4. the freeze/readonly status does not make the cell uneditable, that is I can still change the cell's contents
  5. with a readonly status I can run the cell (as expected).

So, it seems that with a very small notebook the only thing which doesn't work as expected is that the code editing of a readonly/frozen cell is enabled and not disabled as expected.

@odedbd
Copy link
Author

odedbd commented Feb 12, 2017

I tested a few other extensions and with a small notebook all of them show the persisted state as expected. I would think that waiting for the whole notebook to load before running the extensions might be a good idea in general. Perhaps there could be something similar to document ready/loaded javascript events that different extensions could bind on as necessary?

@jcb91
Copy link
Member

jcb91 commented Feb 12, 2017

I would think that waiting for the whole notebook to load before running the extensions might be a good idea in general

Absolutely, that's the proper way to do it in most cases (in some it doesn't make any difference, but any that alter the notebook structure/behaviour should be waiting). I don't think it's especially difficult to implement either, we just haven't checked them all...

@juhasch
Copy link
Member

juhasch commented Feb 12, 2017

Thanks for checking. This was never much of an issue, as notebooks used to be relatively small.
I guess it makes sense to update the extensions now.

@kukanya
Copy link
Contributor

kukanya commented Feb 12, 2017

Hmm, I think I remember implementing a work-around that guaranteed the state of cells will be set once notebook is loaded. I'll double-check it though.

the freeze/readonly status does not make the cell uneditable, that is I can still change the cell's contents

I can't reproduce this. @jcb91, @juhasch, could you guys try to reproduce the issue?

@kukanya
Copy link
Contributor

kukanya commented Feb 12, 2017

        if (typeof Jupyter.notebook === "undefined") {
            events.on("notebook_loaded.Notebook", initialize_states);
        } else {
            initialize_states();
        }

Here's the aforementioned work-around (it was suggested by @jcb91, if I remember correctly), but seems like it isn't enough then?

@jcb91
Copy link
Member

jcb91 commented Feb 12, 2017

yeah, I think the notebook gets defined before it's finished loading, at least in newer notebook versions. Also, the notebook_loaded event can get fired by loading a checkpoint, so I've attempted to make a more robust solution (see #883 for an example), which for free would read

events.on("notebook_loaded.Notebook", initialize_states);
if (Jupyter.notebook !== undefined && Jupyter.notebook._fully_loaded) {
    // notebook already loaded, so we missed the event, so update all
    initialize_states();
}

@kukanya
Copy link
Contributor

kukanya commented Feb 12, 2017

I'm making some patches now, could you guys give me an idea of how can I trace it when user edits cell metadata directly?

@kukanya kukanya mentioned this issue Feb 12, 2017
@jcb91
Copy link
Member

jcb91 commented Feb 13, 2017

didn't actually mean to close this, as it sounds like @odedbd is still having some issue unreletad to the one fixed in #884

  1. the freeze/readonly status does not make the cell uneditable, that is I can still change the cell's contents

@jcb91 jcb91 reopened this Feb 13, 2017
@odedbd
Copy link
Author

odedbd commented Feb 13, 2017

@jcb91 - thanks, I can confirm that the persistence between closing-reopening/refreshing the notebook is now working for me, but the cell content is still editable even when cell is marked as readonly/frozen.

@odedbd
Copy link
Author

odedbd commented Feb 14, 2017

I just retested and not the freeze functionality seems to be working fine. I cleared browser cache before my previous test, and did not do so since, so I am unsure what changed since my previous post. I will close this now and if the problem returns I will open a new issue, as that is a separate problem from the persistence.

Thanks for your help everybody! I am pushing my co-workers to use Jupyter notebooks in our team and the contrib nbextensions were a huge help with that. The super quick and helpful response to issues is an even bigger help.

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 a pull request may close this issue.

5 participants