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

save widgets after execute #779

Closed
wants to merge 6 commits into from

Conversation

@scottdraves
Copy link

scottdraves commented Mar 18, 2018

finished @ricklupton's PR

ricklupton and others added 4 commits Dec 10, 2017
Currently default values are not removed and binary buffers are not saved.

(updated to conform to widget state schema)
@scottdraves scottdraves changed the title save widget's after execute save widgets after execute Mar 18, 2018
@scottdraves

This comment has been minimized.

Copy link
Author

scottdraves commented Mar 19, 2018

Looking into fixing on Python 2.7...

@mpacer

This comment has been minimized.

Copy link
Member

mpacer commented Mar 19, 2018

@jasongrout So is this your preferred mechanism for having widgets in exported contexts? Rather than with the custom mimetype and using the cdn approach?


try:
nb, resources = super(ExecutePreprocessor, self).preprocess(nb, resources)
finally:
self.kc.stop_channels()
self.km.shutdown_kernel(now=self.shutdown_kernel == 'immediate')
if self.widget_state:

This comment has been minimized.

Copy link
@mpacer

mpacer Mar 19, 2018

Member

Can you isolate this functionality in a separate method?

setup.py Outdated
@@ -199,7 +199,7 @@ def run(self):
]

extra_requirements = {
'test': ['pytest', 'pytest-cov', 'ipykernel', 'jupyter_client>=4.2'],
'test': ['pytest', 'pytest-cov', 'ipykernel', 'jupyter_client>=4.2', 'ipywidgets'],

This comment has been minimized.

Copy link
@mpacer

mpacer Mar 19, 2018

Member

Does this need to be a specific version?

This comment has been minimized.

Copy link
@jasongrout

jasongrout Mar 19, 2018

Member

should probably be at least >=7


wdata = output_nb['metadata']['widgets'] \
['application/vnd.jupyter.widget-state+json']
for k in model_ids:

This comment has been minimized.

Copy link
@mpacer

mpacer Mar 19, 2018

Member

Nice tests!

]
}
],
"metadata": {},

This comment has been minimized.

Copy link
@jasongrout

jasongrout Mar 19, 2018

Member

Where is the widget state for the widget view above?

@jasongrout

This comment has been minimized.

Copy link
Member

jasongrout commented Mar 19, 2018

@jasongrout So is this your preferred mechanism for having widgets in exported contexts? Rather than with the custom mimetype and using the cdn approach?

I glanced through it, and it looks all right for storing widget state in a notebook. For a notebook, the widget views are stored as custom mimetypes, and the widget state is stored in the notebook metadata. The cdn is used by an html output, for example, to actually get the js code to render the widget views.

lmitusinski added 2 commits Mar 22, 2018
ipywidgets min version changed to 7
@scottdraves

This comment has been minimized.

Copy link
Author

scottdraves commented Mar 29, 2018

@mpacer @jasongrout should be ready to go.

@blink1073

This comment has been minimized.

Copy link
Member

blink1073 commented Aug 29, 2018

@jasongrout, I'm about to cut 5.4, is this okay in your mind?

@blink1073 blink1073 modified the milestones: 5.4, 5.5 Aug 29, 2018
@Juanlu001

This comment has been minimized.

Copy link

Juanlu001 commented Sep 23, 2018

This needs a rebase, thanks for tackling it @scottdraves!

@MSeal

This comment has been minimized.

Copy link
Collaborator

MSeal commented Apr 15, 2019

I believe this is now completed via #900 -- please reopen and rebase if you think there's anything that got missed.

@MSeal MSeal closed this Apr 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.