Fix for compat for ipywidgets and ipynb #196
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Obviously, I don't know what motivate the comment just below, and we would need to remove this comment if we go with the PR. |
|
This is unexpected: as far as I understand, if there are widgets in the notebook, the notebook must contain widget state. Do you have a minimal reproducible example? Can you write the notebook to disk around that point and share it? |
|
Sure, let me find the notebook that causes the issue, so we can clarify the
root cause of the issue.
…On Thu, 10 Feb 2022 at 14:56, Anton Akhmerov ***@***.***> wrote:
This is unexpected: as far as I understand, if there are widgets in the
notebook, the notebook must contain widget state. Do you have a minimal
reproducible example? Can you write the notebook to disk around that point
and share it?
—
Reply to this email directly, view it on GitHub
<#196 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AJPMSPHKB5M2RCIXCDPAD6LU2PG3TANCNFSM5OA22YKQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Actually, the notebooks had widgets which had failed to render due to the
fact the notebook had apparently been rerendered by someone who did
not have the extension installed. We had quite a lot of notebooks in our
sphinx and we had not noticed this issue.
Whilst the exception has pointed to an issue in the notebook,
a crash with KeyError when trying to render a large doc repo with sphinx is
not really expected and
the message not really helpful to most users to understand the cause of the
issue.
Sphinx generally does not crash for tech errors, broken references, etc..
it simply issues warnings.
I am thinking that it would be nicer here to be able raise a warning - that
can be controlled by warnings control or to directly log a message.
I will see if I can amend my PR to do this - as long as you are fine with
this change.
On Thu, 10 Feb 2022 at 16:06, Bertrand Nouvel ***@***.***>
wrote:
… Sure, let me find the notebook that causes the issue, so we can clarify
the root cause of the issue.
On Thu, 10 Feb 2022 at 14:56, Anton Akhmerov ***@***.***>
wrote:
> This is unexpected: as far as I understand, if there are widgets in the
> notebook, the notebook must contain widget state. Do you have a minimal
> reproducible example? Can you write the notebook to disk around that point
> and share it?
>
> —
> Reply to this email directly, view it on GitHub
> <#196 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AJPMSPHKB5M2RCIXCDPAD6LU2PG3TANCNFSM5OA22YKQ>
> .
> Triage notifications on the go with GitHub Mobile for iOS
> <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
> or Android
> <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
>
> You are receiving this because you authored the thread.Message ID:
> ***@***.***>
>
|
|
I agree that sphinx generally tends to not abort on data inconsistencies. Here, however, it is not clear to me how the error may occur and whether it isn't an actual bug. I believe that jupyter widgets must always write widget state whenever widgets are present. If that is correct, then this not happening would be an actual bug in the widget library and the way it communicates with the kernel, and shouldn't be treated like a user input problem. Basically something should create a widgets metadata entry in the notebook, but then not write widget state into this metadata. I don't think the error message in the PR corresponds to the situation I described, and therefore wouldn't help debug what is happening. Can you share more information about the conditions under which the problem occurs? |
|
Hi Anton,
You are right that normally the widget state should have created:
The typical cells that were causing issues in the notebook were structured
like this:
- {
- "data": {
- "application/vnd.jupyter.widget-view+json": {
- "model_id": "509796ddf1dd47c19545c4eac2da15e9",
- "version_major": 2,
- "version_minor": 0
- },
- "text/html": [
- "<p>Failed to display Jupyter Widget of type
<code>HBox</code>.</p>\n",
- "<p>\n",
- " If you're reading this message in the Jupyter Notebook or
JupyterLab Notebook, it may mean\n",
- " that the widgets JavaScript is still loading. If this message
persists, it\n",
- " likely means that the widgets JavaScript library is either not
installed or\n",
- " not enabled. See the <a href=\"
https://ipywidgets.readthedocs.io/en/stable/user_install.html\">Jupyter\n",
- " Widgets Documentation</a> for setup instructions.\n",
- "</p>\n",
- "<p>\n",
- " If you're reading this message in another frontend (for example,
a static\n",
- " rendering on GitHub or <a href=\"https://nbviewer.jupyter.org/\
">NBViewer</a>),\n",
- " it may mean that your frontend doesn't currently support
widgets.\n",
- "</p>\n"
--
- ]
- },
As indicated it suggests that the python part of ipywidgets was installed
but not the javascript part when the notebook was initially rendered.
This is certainly an issue with the notebook - the issue for jupyter-sphinx
is just to make it clear to the user that something is wrong inside of the
notebook.
You obviously know the code base much more than I do - you may find a
better way to help users to identify the nature of the issue than the one I
have suggested.
For instance, I was not sure if there was a way to indicate the name of the
notebook from where the exception was raised - being able to point at the
notebook and to put
an exact error message would already help a lot.
Kind regards,
Bertrand
…On Sat, 12 Feb 2022 at 14:51, Anton Akhmerov ***@***.***> wrote:
I agree that sphinx generally tends to not abort on data inconsistencies.
Here, however, it is not clear to me how the error may occur and whether it
isn't an actual bug. I believe that jupyter widgets must always write
widget state whenever widgets are present. If that is correct, then this
not happening would be an actual bug in the widget library and the way it
communicates with the kernel, and shouldn't be treated like a user input
problem. Basically *something* should create a widgets metadata entry in
the notebook, but then not write widget state into this metadata.
I don't think the error message in the PR corresponds to the situation I
described, and therefore wouldn't help debug what is happening. Can you
share more information about the conditions under which the problem occurs?
—
Reply to this email directly, view it on GitHub
<#196 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AJPMSPBOAFM5567Y4V6EGO3U2ZXYLANCNFSM5OA22YKQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
This is weird: jupyter-sphinx does execution using nbclient, which doesn't have a javascript side and never talks with javascript. How are you using a notebook file wit jupyter-sphinx in the first place? |
|
My bad that was not the issue - the issue is in the metadata of the
notebook not on the cell that are rendered:
So, using jq. I can see two type of headers on with the mimetype before the
state in widgets ( which I believe are aligned with the code:
```
{
"kernelspec": {
"display_name": "Python 3",
"language": "python",
"name": "python3"
},
"language_info": {
"codemirror_mode": {
"name": "ipython",
"version": 3
},
"file_extension": ".py",
"mimetype": "text/x-python",
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.6.6"
},
"widgets": {
"application/vnd.jupyter.widget-state+json": {
"state": {
"0003c36486af462eaad389ed91fe3314": {
"model_module": ***@***.***/base",
"model_module_version": "1.0.0",
"model_name": "LayoutModel",
"state": {}
},
"00376cf2c7114633ba2c9fbec3e2e1c5": {
"model_module": ***@***.***/controls",
"model_module_version": "1.1.0",
"model_name": "VBoxModel",
"state": {
"_dom_classes": [
"widget-interact"
],
"children": [
"IPY_MODEL_5dc9df01b83045f5b3e93d95b07eccc7",
"IPY_MODEL_41b984db2c8647a09ec4a7c89033c542"
],
"layout": "IPY_MODEL_ed159285a2204ed18179675c03bc9605"
}
},
"003f9cab73ee4dd4add71f6e1e1b3d50": {
"model_module": ***@***.***/output",
"model_module_version": "1.0.0",
"model_name": "OutputModel",
"state": {
"layout": "IPY_MODEL_72632dec96e341329134575d3dc8aaf5"
}
...
```
And some other notebooks I can see the state is integrated directly,
```
{
"kernelspec": {
"display_name": "Python 3",
"language": "python",
"name": "python3"
},
"language_info": {
"codemirror_mode": {
"name": "ipython",
"version": 3
},
"file_extension": ".py",
"mimetype": "text/x-python",
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.6.6"
},
"widgets": {
"state": {
"e06d5fafe05e421ca7a21d167f5eaa85": {
"views": [
{
"cell_index": 40
}
]
}
},
"version": "1.2.0"
}
}
```
So they must have been saved by different versions of jupyter / ipywidgets
- one of them being incompatible with the assumptions being made in
jupyter-sphinx.
Bertrand
On Mon, 14 Feb 2022 at 14:21, Bertrand Nouvel ***@***.***>
wrote:
… Hi Anton,
You are right that normally the widget state should have created:
The typical cells that were causing issues in the notebook were structured
like this:
- {
- "data": {
- "application/vnd.jupyter.widget-view+json": {
- "model_id": "509796ddf1dd47c19545c4eac2da15e9",
- "version_major": 2,
- "version_minor": 0
- },
- "text/html": [
- "<p>Failed to display Jupyter Widget of type
<code>HBox</code>.</p>\n",
- "<p>\n",
- " If you're reading this message in the Jupyter Notebook or
JupyterLab Notebook, it may mean\n",
- " that the widgets JavaScript is still loading. If this message
persists, it\n",
- " likely means that the widgets JavaScript library is either not
installed or\n",
- " not enabled. See the <a href=\"
https://ipywidgets.readthedocs.io/en/stable/user_install.html\
">Jupyter\n",
- " Widgets Documentation</a> for setup instructions.\n",
- "</p>\n",
- "<p>\n",
- " If you're reading this message in another frontend (for
example, a static\n",
- " rendering on GitHub or <a href=\"https://nbviewer.jupyter.org/\
">NBViewer</a>),\n",
- " it may mean that your frontend doesn't currently support
widgets.\n",
- "</p>\n"
--
- ]
- },
As indicated it suggests that the python part of ipywidgets was installed
but not the javascript part when the notebook was initially rendered.
This is certainly an issue with the notebook - the issue for
jupyter-sphinx is just to make it clear to the user that something is wrong
inside of the notebook.
You obviously know the code base much more than I do - you may find a
better way to help users to identify the nature of the issue than the one I
have suggested.
For instance, I was not sure if there was a way to indicate the name of
the notebook from where the exception was raised - being able to point at
the notebook and to put
an exact error message would already help a lot.
Kind regards,
Bertrand
On Sat, 12 Feb 2022 at 14:51, Anton Akhmerov ***@***.***>
wrote:
> I agree that sphinx generally tends to not abort on data inconsistencies.
> Here, however, it is not clear to me how the error may occur and whether it
> isn't an actual bug. I believe that jupyter widgets must always write
> widget state whenever widgets are present. If that is correct, then this
> not happening would be an actual bug in the widget library and the way it
> communicates with the kernel, and shouldn't be treated like a user input
> problem. Basically *something* should create a widgets metadata entry in
> the notebook, but then not write widget state into this metadata.
>
> I don't think the error message in the PR corresponds to the situation I
> described, and therefore wouldn't help debug what is happening. Can you
> share more information about the conditions under which the problem occurs?
>
> —
> Reply to this email directly, view it on GitHub
> <#196 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AJPMSPBOAFM5567Y4V6EGO3U2ZXYLANCNFSM5OA22YKQ>
> .
> Triage notifications on the go with GitHub Mobile for iOS
> <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
> or Android
> <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
>
> You are receiving this because you authored the thread.Message ID:
> ***@***.***>
>
|
|
I believe the lower format was used in |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We had issues with some serialised state from ipywidgets in notebook.
It was working fine when we using
nbsphinxand old pinned dependencies - however when updating to latest sphinx and packages an exception was raised here. It seems it was excepted but that the wrong exception was caught - and that the exact type of exception to capture depends on other dependencies. I believe this change should be reasonable.