Add support for getters

[bnmajor]

-- WIP --

Guided by the items discussed in #568 this branch attempts to add support for getter functions. Adapting approaches in the ipython_blocking and jupyter-ui-poll libraries the general approach is to:

• Temporarily overwrite execute_request handler in IPython in order to collect and queue requests
• Once the viewer is created the background thread calls execute_next_request to start the cycle of processing the remaining cells
• Pop requests off one at a time and inspect for getters
  • If getters are found we create a future and attach a callback that sets the result once it is received. Once all results have resolved the cell is re-run so that it may return the results.
  • If no getters are found the request is just run as usual

Issues:

• Currently the user namespace is not being updated when the cell is re-run properly so cell output is always wrong

[review-notebook-app[bot]]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[bnmajor]

This approach is making progress but is still a bit naive and has a lot of additional cases/needs to take into account. Starting a list here of key concerns and shortcomings so that they are addressed as this moves forward:

• [ ] We are currently running getters twice - once before and once during cell execution. This approach needs to be re-thought as there are plenty of use cases where this would not return the correct value. ex:

  print(viewer.get_axes_enabled()) # False by default
  viewer.set_axes_enabled(True)
  print(viewer.get_axes_enabled()) # Should be True, will return False

• [ ] Need to confirm that none of the approaches that we settle on inadvertently affect the notebook history or output
• [ ] Currently using regex to identify getters. There are plenty of cases where a getter or setter may not be called directly so this is not a viable approach.

[thewtex]

Todos

• Multiple viewers
• Test notebook: 1) set, get, throws exceptions
• Intercept getters during execution instead of after

which should help with all three points above.

-> merge

Create another issue as needed for setting cell output with a getter output.

[thewtex]

@bnmajor this timeout only occurs when the tests are executed through nbmake?

@alex-treebeard are notebook events processed in a different way when executed through nbmake?

[thewtex]

@alex-treebeard suggested that there may be a to resolve via the way nbmake uses nbclient.

[bnmajor]

@thewtex So it seems like the nbmake issue is not actually with the getters/setters but with the fact that in testing the itk-viewer is never actually created (so any cells with getters/setters timeout waiting for it to be created before being run). If I use the --overwrite flag to see the results produced from testing there is no embedded screenshot which seems to backup this theory....

[thewtex]

@bnmajor oh, good catch! :bulb:

Indeed

This widget rendering is not performed against a browser during execution

@oeway any ideas / suggestions / pointers to have ImJoy talk to a headless chromium browser?

[alex-treebeard]

@thewtex been doing a bit of reading around widgets. Is it necessary to load the browser in order to initialise the ITK widget?

What needs to run in order to unblock the getters from executing against an initialised viewer object?

[thewtex]

@alex-treebeard

Is it necessary to load the browser in order to initialise the ITK widget?

It is not necessary to import and run the Python code. However, a browser is necessary for the graphical context and JavaScript environment to perform renderings and work with the client-side widget state.

What needs to run in order to unblock the getters from executing against an initialised viewer object?

One way or another, we need to be talking to, e.g., Chrome / Electron in Headless Mode, when running in nbmake / nbclient.

[alex-treebeard]

@thewtex have you checked out galata?

It's possible to stand up a jupyter lab instance in a test environment, then we could potentially run nbmake against its kernel.

[thewtex]

@alex-treebeard great idea!!

Yes, we could use a version of nb_run.py that uses galata and something along the lines of:

https://github.com/jupyterlab/jupyterlab/blob/2250b956294558de58a99e4a26286270654eb416/galata/test/galata/notebook.spec.ts#L91-L115

[thewtex]

Let's merge this, then, in separate PR's

• Document getters limitations in sphinx
• Address the JS testing issues