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

_get_comm_info() is needed for jupyter-widget set_state() but nteract doesn't have it implemented yet. #101

Open
kjcho-msft opened this issue Mar 8, 2022 · 2 comments

Comments

@kjcho-msft
Copy link

kjcho-msft commented Mar 8, 2022

Looking for a way to add the functionality to save/restore ipywidget state using nteract widget manager.

See the following implementation of set_state() in jupyter-widget and it needs _get_comm_info() but _get_comm_info() is not implemented in nteract.

My question is, can nteract add _get_comm_info implementation?

<set_state() in jupyter-widget>
set_state(state: IManagerState): Promise<WidgetModel[]> {
// Check to make sure that it's the same version we are parsing.
if (!(state.version_major && state.version_major <= 2)) {
throw 'Unsupported widget state format';
}
const models = state.state as any;
// Recreate all the widget models for the given widget manager state.
const all_models = this._get_comm_info().then((live_comms) => {
/* Note: It is currently safe to just loop over the models in any order,
given that the following holds (does at the time of writing):
1: any call to new_model with state registers the model promise (e.g. with register_model)
synchronously (before it's first await statement).
2: any calls to a model constructor or the set_state method on a model,
happens asynchronously (in a then clause, or after an await statement).
Without these assumptions, one risks trying to set model state with a reference
to another model that doesn't exist yet!
*/

<_get_comm_info() in nteract>
_get_comm_info(): Promise<{}> {
throw new Error("_get_comm_info is not implemented!");
}

@captainsafia
Copy link
Member

This felt oddly familiar so I took a look at this and noticed we had previously filed nteract/nteract#4675.

It looks like the lack of a __get_comm_info implementation was intentional based on the previous design and the way that that the ipywidgets implementation interacts with the state.

Was there a particular motivation for filing this issue?

cc: @miduncan

@kjcho-msft
Copy link
Author

kjcho-msft commented May 5, 2022

I found _get_comm_info implementation is needed for restoring ipywidget state feature based on set_state() API in jupyter-widget manager-base.

You said you didn't add it intentionally. Does it mean there is a way to get the feature enabled without implementing get_comm_info?

@captainsafia, please provide some guidance on what would be right way to add support for restoring ipywidget state using nteract.

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

No branches or pull requests

2 participants