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

evaluating ipywidget code block in nbdev_preview does not result in output #1042

Closed
tylere opened this issue Sep 12, 2022 · 15 comments
Closed

Comments

@tylere
Copy link
Contributor

tylere commented Sep 12, 2022

(This is a follow up to the Nbdev v2 & Jupyter Widgets discussion started on the nbdev forum.)

I am trying to get ipywidget output to render in nbdev_preview, with an eventual goal of getting it to render on GitHub pages. I am using (hopefully correctly) the master versions of nbdev, quarto, fastcore, execnb.

To reproduce:

Clone the test repository

https://github.com/tylere/test-evaluate-code-3

Setup a Local Environment setup

conda create -n test-evaluate-code-3 -c conda-forge python=3.10 gh jupyterlab jupyterlab-git ipywidgets ipyleaflet
conda activate test-evaluate-code-3
cd test-evaluate-code-3
# Install fastai dependencies
pip install -e ../../fastai/fastcore/
pip install -e ../../fastai/execnb/
pip install -e ../../fastai/nbdev/
nbdev_new
nbdev_install_hooks
# Install dev version of quarto
# https://github.com/quarto-dev/quarto-cli#development-version
ln -s /Users/tylere/Documents/GitHub/quarto-dev/quarto-cli/package/dist/bin/quarto /Users/tylere/miniconda3/envs/test-evaluate-code-3/bin/quarto
pip install -e '.[dev]'

Prerelease dependency versions

(test-evaluate-code-3) tylere-macbookpro5:test-evaluate-code-3 tylere$ (cd ../../fastai/fastcore && git pull && git log -1)
Already up to date.
commit 0ee5b7f52a6b8d81691046d73555d56a25fbafac (HEAD -> master, origin/master, origin/HEAD)
Merge: f7e81b8 8fdfd81
Author: Jeremy Howard <j@fast.ai>
Date:   Mon Sep 12 08:59:13 2022 +1000

    Merge pull request #486 from seeM/fix-http-exceptions
    
    fix: error in IPython while handling `HTTP4xxClientError`
(test-evaluate-code-3) tylere-macbookpro5:test-evaluate-code-3 tylere$ (cd ../../fastai/execnb && git pull && git log -1)
Already up to date.
commit 25f3a2f94eb0ea5b9c88303dfd6b9130caa7a029 (HEAD -> master, tag: 0.1.3, origin/master, origin/HEAD)
Author: Jeremy Howard <j@fast.ai>
Date:   Sat Sep 3 13:13:53 2022 +1000

    release
(test-evaluate-code-3) tylere-macbookpro5:test-evaluate-code-3 tylere$ (cd ../../fastai/nbdev && git pull && git log -1)
Already up to date.
commit 8b354dbed937005b500197431cc43c85cdf37d09 (HEAD -> master, origin/master, origin/HEAD)
Merge: 98fa120 6699aee
Author: Hamel Husain <hamel.husain@gmail.com>
Date:   Sun Sep 11 20:27:55 2022 -0700

    Merge pull request #1039 from fastai/nb-add
    
    refactor best practices intro
(test-evaluate-code-3) tylere-macbookpro5:test-evaluate-code-3 tylere$ (cd ../../fastai/nbdev && git pull && git log -1)
Already up to date.
commit 8b354dbed937005b500197431cc43c85cdf37d09 (HEAD -> master, origin/master, origin/HEAD)
Merge: 98fa120 6699aee
Author: Hamel Husain <hamel.husain@gmail.com>
Date:   Sun Sep 11 20:27:55 2022 -0700

    Merge pull request #1039 from fastai/nb-add
    
    refactor best practices intro
(test-evaluate-code-3) tylere-macbookpro5:test-evaluate-code-3 tylere$ (cd ../../quarto-dev/quarto-cli/ && git pull && git log -1)
Already up to date.
commit cb5343c248387e7e0c46b5e90c72479d0615a59b (HEAD -> main, tag: v1.2.95, origin/main, origin/HEAD)
Author: Charles Teague <cteague@gmail.com>
Date:   Mon Sep 12 13:44:01 2022 -0400

    Pass meta to shortcodes without messing with it
    
    Fixes #2407 (part 1 - pass meta through cleanly)

View the output in JupyterLab

Screen Shot 2022-09-12 at 1 43 35 PM

Preview the output

nbdev_preview
Note that the datetime printout rendered correctly, but the two ipywidgets do not render.

Screen Shot 2022-09-12 at 1 43 38 PM

Clues

The nbdev_preview output warns "Malformed Jupyter Output Display Data found"

(test-evaluate-code-3) tylere-macbookpro5:test-evaluate-code-3 tylere$ nbdev_preview
Check file:///Users/tylere/Documents/GitHub/quarto-dev/quarto-cli/src/quarto.ts
Preparing to preview
[1/1] 01_test_ipywidget_evaluate.ipynb

Starting python3 kernel...Done

Executing '01_test_ipywidget_evaluate.ipynb'
  Cell 1/6...Done
  Cell 2/6...Done
  Cell 3/6...Done
  Cell 4/6...Done
  Cell 5/6...Done
  Cell 6/6...Done

WARNING: Malformed Jupyter Output Display Data found:
WARNING: {"model_id":"1149bbef449c4481bf183fb503c7079d","version_major":2,"version_minor":0}
WARNING: Malformed Jupyter Output Display Data found:
WARNING: {"model_id":"a4ac7997ebed4a8b969b5827cb3af393","version_major":2,"version_minor":0}

Watching files for changes
Browse at http://localhost:3000/01_test_ipywidget_evaluate.html
GET: /01_test_ipywidget_evaluate.html
@rxavier
Copy link

rxavier commented Sep 13, 2022

I'm having the same issue with ipywidget-enabled tqdm progress bars. nbdev_preview throws WARNING: Malformed Jupyter Output Display Data found: and no progress bars can be seen in the quarto output, even though they are fine in the notebook.

@jph00
Copy link
Member

jph00 commented Sep 14, 2022

I'm having the same issue with ipywidget-enabled tqdm progress bars. nbdev_preview throws WARNING: Malformed Jupyter Output Display Data found: and no progress bars can be seen in the quarto output, even though they are fine in the notebook.

Please open a new issue with full details to repro your problem -- adding to an existing issue means we don't have the info we need to help, and your issue won't be tracked separately.

@jph00
Copy link
Member

jph00 commented Sep 15, 2022

@seeM
Copy link
Contributor

seeM commented Sep 15, 2022

I've reproduced this with a tiny example in quarto (no nbdev), tracking in the quarto repo:

@jph00
Copy link
Member

jph00 commented Sep 15, 2022

@tylere the upstream quarto issue is fixed in prerelease now - can you see if that helps?

@tylere
Copy link
Contributor Author

tylere commented Sep 16, 2022

Wahoo! I now see the widget output in the preview!

drawing

and they now show up on GitHub Pages:

drawing

For completeness, here is the current versions of the libraries:

(test-evaluate-code-3) tylere-macbookpro5:test-evaluate-code-3 tylere$ (cd ../../fastai/fastcore && git pull && git log -1)
Already up to date.
commit ac83d6be6229478dc516415407379ac8c534ece0 (HEAD -> master, tag: 1.5.27, origin/master, origin/HEAD)
Author: Jeremy Howard <j@fast.ai>
Date:   Fri Sep 16 05:46:33 2022 +1000

    release
(test-evaluate-code-3) tylere-macbookpro5:test-evaluate-code-3 tylere$ (cd ../../fastai/ghapi && git pull && git log -1)
Already up to date.
commit 9dceb6c0d7a240bb2b936ea17acf36de698e645a (HEAD -> master, tag: 1.0.3, origin/master, origin/HEAD)
Author: Jeremy Howard <j@fast.ai>
Date:   Tue Sep 13 12:26:00 2022 +1000

    release
(test-evaluate-code-3) tylere-macbookpro5:test-evaluate-code-3 tylere$ (cd ../../fastai/execnb && git pull && git log -1)
Already up to date.
commit 25f3a2f94eb0ea5b9c88303dfd6b9130caa7a029 (HEAD -> master, tag: 0.1.3, origin/master, origin/HEAD)
Author: Jeremy Howard <j@fast.ai>
Date:   Sat Sep 3 13:13:53 2022 +1000

    release
(test-evaluate-code-3) tylere-macbookpro5:test-evaluate-code-3 tylere$ (cd ../../fastai/nbdev && git pull && git log -1)
Already up to date.
commit b5019418d58140e4d6f3af4c3d62043335f3ef06 (HEAD -> master, tag: 2.3.3, origin/master, origin/HEAD)
Author: Jeremy Howard <j@fast.ai>
Date:   Fri Sep 16 10:21:34 2022 +1000

    release
(test-evaluate-code-3) tylere-macbookpro5:test-evaluate-code-3 tylere$ (cd ../../quarto-dev/quarto-cli/ && git pull && git log -1)
Already up to date.
commit 603e94485339497e2d0f232a1acfaf3895a8feeb (HEAD -> main, origin/main, origin/HEAD)
Author: Charles Teague <cteague@gmail.com>
Date:   Thu Sep 15 17:16:10 2022 -0400

    only cleanup cached css file if it exists

Thank you! I think it is fine to close this now.

@jph00
Copy link
Member

jph00 commented Sep 16, 2022

Yay! :D

@jph00 jph00 closed this as completed Sep 16, 2022
@tylere
Copy link
Contributor Author

tylere commented Sep 19, 2022

Looks like I was celebrating too soon...
While the widget rendering works on the simple index page/notebook, rendering does not work in notebooks that define a module.

Example repo: https://github.com/tylere/test-evaluate-code-4

Here is the error I receive:

(test-evaluate-code-4) tylere-macbookpro5:test-evaluate-code-4 tylere$ nbdev_preview
NB: From v1.2 `_quarto.yml` is no longer auto-updated. Please remove the `custom_quarto_yml` line from `settings.ini`
Preparing to preview
[1/1] 00_core.ipynb

Starting python3 kernel...Done

Executing '00_core.ipynb'
  Cell 1/1...ERROR: 

An error occurred while executing the following cell:
------------------
show_doc(foo)
------------------

---------------------------------------------------------------------------
NameError                                 Traceback (most recent call last)
Cell In [1], line 1
----> 1 show_doc(foo)

NameError: name 'show_doc' is not defined
NameError: name 'show_doc' is not defined

Here are the library versions used:

deps/execnb/
0e4c476 (HEAD -> master, origin/master, origin/HEAD) fix CI
deps/fastcore/
2ccab20 (HEAD -> master, origin/master, origin/HEAD) bump
deps/ghapi/
70fa621 (HEAD -> master, origin/master, origin/HEAD) CHANGELOG.md
deps/nbdev/
d26ce6e (HEAD -> master, origin/master, origin/HEAD) Merge pull request #1096 from fastai/migrate-fix
deps/quarto-cli/
97531040b (HEAD, tag: v1.2.131, origin/main, origin/HEAD, main) Merge pull request #2486 from jkseppan/author-localization-fix

@seeM
Copy link
Contributor

seeM commented Sep 19, 2022

IIUC this is a bit of confusion between nbdev and Quarto's processors. When generating documentation, nbdev pre-processes the notebook before Quarto processes it. You can see the nbdev-processed version in the _proc folder -- useful for debugging.

In this case: nbdev removes#|exported cells, adds show_doc(...) cells, and removes #|hide cells. Since the from nbdev.showdoc import * cell has #|hide it's removed and show_doc isn't available when Quarto tries to then execute the notebook.

I'm not yet sure about the best solution.. Maybe nbdev could mark the cell as eval: false after execnb runs show_doc. @hamelsmu @jph00 what do you think?


BTW, why is the execute: true needed in the first place? Is it to ensure that the widget state gets added to the notebook? Maybe a better solution is to enable nbdev to do that in its own processing rather than deferring to Quarto.

@tylere
Copy link
Contributor Author

tylere commented Sep 19, 2022

BTW, why is the execute: true needed in the first place? Is it to ensure that the widget state gets added to the notebook?

I think it is needed... at least I don't know how to get the widgets to render correctly without it. But I would be happy to be corrected.

According to the Quarto docs Output Options, the code block directives should override the frontmatter instructions. So maybe a solution is for nbdev to generate appropriate cell directives (#|execute:false ?) for all cells you want to ensure that quarto should not execute (like those containing show_doc()).

@jph00
Copy link
Member

jph00 commented Sep 19, 2022 via email

@tylere
Copy link
Contributor Author

tylere commented Sep 20, 2022

With JupyterLab's "Save Widget State Automatically" enabled, the UI does save the widget state, but then nbdev's default "Jupyter hook" behavior strips the widget state.

If I set jupyter_hooks = False in settings.ini, the widget state is stored in the ipynb file, and nbdev_preview displays the widget output. Good, but I'm guessing I'll miss out on the beneficial parts of the Jupyter hooks.

I can customize the notebook cleaning by setting allowed_metadata_keys = widgets, and the widget state persists. Which seems like a good solution.

@jph00
Copy link
Member

jph00 commented Sep 20, 2022

Super! BTW that cleaning thing we've filed as a bug: #1069 . cc @seeM

@seeM
Copy link
Contributor

seeM commented Sep 21, 2022

#1069 is now fixed in latest master.

@tylere
Copy link
Contributor Author

tylere commented Oct 25, 2022

This is currently labeled as "waiting for response" but I think that can be removed now.

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

4 participants