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

WIP: add support for ipywidgets #2

Closed
wants to merge 4 commits into from
Closed

Conversation

jbweston
Copy link
Owner

No description provided.

cornhundred and others added 2 commits September 4, 2018 13:29
changed "no kernel is connect" to "no kernel is connected"
@jbweston jbweston changed the title add support for ipywidgets WIP: add support for ipywidgets Oct 17, 2018
@jbweston
Copy link
Owner Author

jbweston commented Oct 17, 2018

TODO

  • Add ipywidgets javascript (preferably by embedding it; we maybe don't want to load stuff from the web)
  • Ensure that this works when calling display() (only tested when a cell evaluates to a widget)

jupyter_sphinx/execute.py Outdated Show resolved Hide resolved
jupyter_sphinx/execute.py Outdated Show resolved Hide resolved
jupyter_sphinx/execute.py Outdated Show resolved Hide resolved
jupyter_sphinx/execute.py Outdated Show resolved Hide resolved
jupyter_sphinx/execute.py Outdated Show resolved Hide resolved
nb, resources = super(ExecutePreprocessor, ep).preprocess(nb, resources)
info_msg = ep._wait_for_reply(ep.kc.kernel_info())
nb.metadata['language_info'] = info_msg['content']['language_info']
return extract_widget_state(ep)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extract_widget_state will only work properly if:

  • The kernel is IPython (or anything running Python more generally, e.g. xonsh).
  • ipywidgets are importable

Therefore calling extract_widget_state should only be done when it's safe.

In absence of multi-language implementation of the Kernel side of jupyter widgets, I believe a relatively safe heuristic for doing this is to check that the outputs contain at least one widget view.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about just wrapping the code to execute in a try/catch block>

jupyter_sphinx/execute.py Outdated Show resolved Hide resolved
nb, resources = super(ExecutePreprocessor, ep).preprocess(nb, resources)
info_msg = ep._wait_for_reply(ep.kc.kernel_info())
nb.metadata['language_info'] = info_msg['content']['language_info']
return extract_widget_state(ep)
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to be more careful here. Widgets are only a thing for Python kernels

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose the widget protocol could be used from kernels of other languages, but I've never seen this.

We can support only extracting widget state from IPython kernels for the moment, and implement others later if there is demand.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minimally we would need to only extract if the kernel is a python one.

jupyter_sphinx/execute.py Outdated Show resolved Hide resolved
jupyter_sphinx/execute.py Outdated Show resolved Hide resolved
jupyter_sphinx/execute.py Outdated Show resolved Hide resolved
jupyter_sphinx/execute.py Outdated Show resolved Hide resolved
jupyter_sphinx/execute.py Outdated Show resolved Hide resolved
nb, resources = super(ExecutePreprocessor, ep).preprocess(nb, resources)
info_msg = ep._wait_for_reply(ep.kc.kernel_info())
nb.metadata['language_info'] = info_msg['content']['language_info']
return extract_widget_state(ep)
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about just wrapping the code to execute in a try/catch block>

nb, resources = super(ExecutePreprocessor, ep).preprocess(nb, resources)
info_msg = ep._wait_for_reply(ep.kc.kernel_info())
nb.metadata['language_info'] = info_msg['content']['language_info']
return extract_widget_state(ep)
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Soon this will be moot: jupyter/nbconvert#900

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this is targeted for nbconvert 5.5

jupyter_sphinx/execute.py Outdated Show resolved Hide resolved
jupyter_sphinx/execute.py Outdated Show resolved Hide resolved
@basnijholt
Copy link
Collaborator

The url of the external json is not correctly parsed I think:

<script type="application/vnd.jupyter.widget-state+json" src="/home/docs/checkouts/readthedocs.org/user_builds/adaptive/checkouts/latest/docs/source/_build/jupyter_execute/tutorial/tutorial.Learner1D_widget-state.json"></script>

from https://adaptive.readthedocs.io/en/latest/tutorial/tutorial.Learner1D.html

@jbweston
Copy link
Owner Author

@akhmerov unfortunately we can't put the widget state in a separate file because of this: https://github.com/jupyter-widgets/ipywidgets/blob/master/packages/html-manager/src/libembed.ts#L36

because they just take the "innerHTML" from the script tag we can't specify it as a separate file

add an extension for executing code in a jupyter kernel
@SylvainCorlay
Copy link

I merged the main PR to jupyter-sphinx. Would you like to reopen this to master?

@jbweston
Copy link
Owner Author

Closing to reopen against the upstream repo

@jbweston jbweston closed this Nov 25, 2018
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 this pull request may close these issues.

5 participants