MyST-NB icon indicating copy to clipboard operation
MyST-NB copied to clipboard

Inlined HoloViews objects are not rendered

Open maximlt opened this issue 1 year ago • 1 comments

Describe the bug

Hi and thanks for this great library, loving it!

I am very tempted to use more and more features of myst-nb and your ecosystem in general to update the HoloViz documentation. One thing I would like to do, and that is the original motivation of this issue, is to use the Tabs component offered by sphinx-design to display in each tab a different plot. This would be for instance very useful for a library like HoloViews that has a Reference Gallery describing each kind of plot that can be made, as this for 3 plotting backends (Bokeh, Matplotlib and Plotly). We currently have a page for each combination of plot type and plotting backend, and would be happy to reduce that to one page by plot type, having in each page Tabs components that show how the plot renders with the 3 plotting backends.

I've made some experiment with sphinx-design and found out that code-cell directives cannot be nested in other directives. I believe this is the same issue as reported here (https://github.com/executablebooks/jupyter-book/issues/1178) and I understand this is by design, it makes sense to me.

I have then found out about the inlining feature of myst-nb and thought that I could use that as a nice workaround! And indeed starting from a simple example, inlining a simple string variable, I came to the conclusion that this could work.

However, inlining a HoloViews object - the type of object returned by hvPlot - didn't seem to work even if the code-cell displayed it correctly.

image

In case you would try building this example by yourself, I've seen this warning being emitted while the code-cell is rendered (i.e. without having any inline). I don't think it's harmful though. WARNING: skipping unknown output mime type: application/vnd.holoviews_load.v0+json [mystnb.unknown_mime_type]

Happy to provide details on the HoloViews displaying machinery if needed, and if this issue is way too tailored to the HoloViz ecosystem I'd welcome any pointers for us to fix that by ourselves.

Reproduce the bug

  • Create an environment and install the dependencies with pip install pandas hvplot or conda install pandas hvplot
  • Copy the content below in a Markdown file and build the docs with myst-nb:
---
file_format: mystnb
kernelspec:
  name: python3
mystnb:
    execution_mode: 'inline'
---

# Text-based Notebook hvPlot

## Display from a code cell: OK

:::{code-cell} ipython3
import pandas as pd, numpy as np
import hvplot.pandas  # noqa
idx = pd.date_range('1/1/2000', periods=1000)
df  = pd.DataFrame(np.random.randn(1000, 4), index=idx, columns=list('ABCD')).cumsum()

p = df.hvplot()
p
:::


## Display from an inlined eval: not OK

HoloViews obj: {eval}`p`

List your environment

myst-nb                   0.16.0             pyhd8ed1ab_0    conda-forge
myst-parser               0.18.0             pyhd8ed1ab_0    conda-forge

maximlt avatar Aug 12 '22 08:08 maximlt

Thanks for opening your first issue here! Engagement like this is essential for open source projects! :hugs:
If you haven't done so already, check out EBP's Code of Conduct. Also, please try to follow the issue template as it helps other community members to contribute more effectively.
If your issue is a feature request, others may react to it, to raise its prominence (see Feature Voting).
Welcome to the EBP community! :tada:

welcome[bot] avatar Aug 12 '22 08:08 welcome[bot]

Heya, thanks for the feedback and sorry for the delayed response, just finding some time to have a look.

So I run this:

$ mamba create -n mystnb-hvplot pandas hvplot ipykernel myst-nb=0.16
$ conda activate mystnb-hvplot
$ conda list | grep -E "myst|hvplot|pandas|ipython|ipykernel"
hvplot                    0.8.1              pyhd8ed1ab_0    conda-forge
ipykernel                 6.15.2             pyh736e0ef_0    conda-forge
ipython                   8.4.0              pyhd1c38e8_1    conda-forge
myst-nb                   0.16.0             pyhd8ed1ab_0    conda-forge
myst-parser               0.18.0             pyhd8ed1ab_0    conda-forge
pandas                    1.4.4           py310hecf8f37_0    conda-forge
$ mystnb-docutils-pseudoxml --nb-read-as-md=yes tester.md > tester.txt
tester.md:: (INFO/1) Starting inline execution client [mystnb]
tester.md:17: (WARNING/2) No output mime type found from render_priority [mystnb.mime_type]
tester.md:27: (WARNING/2) No output mime type found from render_priority [mystnb.eval]
tester.md:: (INFO/1) Stopping inline execution client [mystnb]
tester.md:: (INFO/1) Executed notebook in 3.95 seconds [mystnb]

You'll note the warnings, and when I looked in the notebook, I found that, when generating a cell with just p, you end up with an initial "empty" output, i.e.

```{code-cell} ipython3
p
```

creates something like

  {
   "cell_type": "code",
   "execution_count": 1,
   "metadata": {},
   "outputs": [
    {
     "data": {},
     "metadata": {},
     "output_type": "display_data"
    },
    {
     "data": {
      "application/vnd.holoviews_exec.v0+json": "...",
      "text/html": "...",
      "text/plain": "..."
      },
     "metadata": {},
     "output_type": "display_data"
    }
   ]
}

Not sure why, a bug in holoview's __repr__x method 🤷

Indeed if I change this line: https://github.com/executablebooks/MyST-NB/blob/23385c29a82374ff5e7b8becf1db4b2e8a42f0f7/myst_nb/core/execute/inline.py#L208

to get the second output:

        return cell.outputs[1] if cell.outputs else None

you get an output.

So the fix may be:

  • firstly to skip warnings for outputs with no data
  • but mainly, perhaps async_eval_expression cannot expect only one output is produced and should return all of them (then we render all)

chrisjsewell avatar Sep 01 '22 10:09 chrisjsewell

Let me know if #440 works 😄

chrisjsewell avatar Sep 01 '22 14:09 chrisjsewell

Sorry for the late feedback! #440 does fix the issue :) HoloViews seems to call IPython's display function to insert a placeholder output to display error messages, if any. Not sure to which extent this is useful or even used, but it doesn't seem feel like a bug either. Anyway, what you've done seems to fix that, thanks a lot.

https://github.com/holoviz/holoviews/blob/632b8b0bd7f73751ab5616b9fd6b1042f7406078/holoviews/plotting/plot.py#L133

maximlt avatar Dec 29 '22 08:12 maximlt