Implement image/png repr #343

martinRenou · 2021-09-14T15:39:02Z

Follow-up of #59

Should fix #150, and fix #16

This seems to properly work in JupyterLab, but we need to push a fix to ipywidgets (and notebook) to have it working in classic Jupyter Notebook as well.

github-actions · 2021-09-14T15:39:16Z

👈 Launch a binder notebook on branch martinRenou/ipympl/text-html

martinRenou · 2021-09-14T15:40:07Z

js/src/mpl_widget.js

@@ -186,7 +188,11 @@ export class MPLCanvasModel extends widgets.DOMWidgetModel {
        this.image.src = image_url;

        // Tell Jupyter that the notebook contents must change.
-        this.send_message('ack');
+        if (!this.acknowledged_rendered) {
+            this.send_message('ack');


Why do we need a round-trip to the front-end??

It looks like we were trying to update the mime bundle with an "update display" message, but honnestly I don't remmember.

We should probably try to keep the PR as simple as possible for now...

I tried implementing it without the round-trip but I wasn't able. I need to look deeper into that.

martinRenou · 2021-09-14T16:12:38Z

IDK what Github uses for rendering Notebooks, but it seems to correctly handle a mix of jupyter-widgets mimetype and image/png mimetype:

nbviewer is not able to render it

SylvainCorlay · 2021-09-14T18:31:24Z

nbviewer is not able to render it

Yes, that is expected as per our discussion on data type priority and the html renderer.

ianhi · 2021-09-14T19:49:28Z

Does this depend on a specific vesion of jupyterlab? On the binder link for this PR I still get the widget mimebundle taking precedence:

martinRenou · 2021-09-15T06:54:37Z

@ianhi I was not aware of this binder link for PRs, is this really using a dev installation of ipympl? I see ipympl in the environment.yml: https://github.com/matplotlib/ipympl/blob/master/environment.yml#L8

martinRenou · 2021-09-15T09:35:48Z

Relies on jupyter/notebook#6181 and jupyter-widgets/ipywidgets#3280 for Jupyter Notebook

ianhi · 2021-09-15T15:59:41Z

@ianhi I was not aware of this binder link for PRs, is this really using a dev installation of ipympl? I see ipympl in the environment.yml: https://github.com/matplotlib/ipympl/blob/master/environment.yml#L8

:/ indeed it seems to be wrong. Ideally the bot that commented gives you a link to binder based on the PR in question.

ianhi · 2021-09-15T16:21:26Z

Does this depend on a specific vesion of jupyterlab?

Testing locally it seems that this doesn't work on jlab 3.0.14 but does on 3.1.12 so something in rendering priorities or response to errors changed between 3.0 and 3.1?

potential problem:

If I understand correctly this approach works because jlab falls back on the lower priority mimetypes when there is an error loading the widget state. This error shows up because the full widget state wasn't saved, but if you check the Save widget state automatically option then the png is never showed and the frontend stalls out waiting for a draw message (it doesn't render until there is some back and forth with pyton) even though I think it should have all the information it needs to draw itself.

(Somewhere I've written down why I think this is happening and how it might be fixed, I'll try to find that)

martinRenou · 2021-09-15T16:29:46Z

This error shows up because the full widget state wasn't saved, but if you check the Save widget state automatically option then the png is never showed

Yes, that should be the intended behavior. We could fix this by including the png data as part of the widget model.

martinRenou · 2021-09-16T09:43:05Z

nbviewer and nbconvert should take this into account if this gets merged: jupyter/nbconvert#1643

ipympl/backend_nbagg.py

ianhi · 2021-09-16T15:33:33Z

ohhh one more thing about this approach. I think we will need to call update_display on every draw call (or more parsimoniously whenever the notebook is saved) because otherwise you don't capture any updates to the figure that happen after the initial display call (panning, zooming, adding elements, manipulating with sliders, etc)

Can widgets listen to notebook save events? I think updating the embedded png on saving is more reasonable than on every draw in order to avoid the flickering you described.

martinRenou · 2021-09-16T15:49:10Z

ohhh one more thing about this approach. I think we will need to call update_display on every draw call (or more parsimoniously whenever the notebook is saved) because otherwise you don't capture any updates to the figure that happen after the initial display call (panning, zooming, adding elements, manipulating with sliders, etc)

Yeah. Though update_display makes the widget flicker and it is quite annoying. We might be able to improve JupyterLab/Notebook to not have this flickering though.
Also this means sending the data twice for each update, once for the widget, and once for the image/png repr, so not really ideal.

Can widgets listen to notebook save events? I think updating the embedded png on saving is more reasonable than on every draw in order to avoid the flickering you described.

Unfortunately not on the Python side. Technically, the kernel doesn't even know what a Notebook is.

I'm tempted to say we could ignore this particular issue for now. As what we have is already quite a big improvement. And other Matplotlib backends are also not able to save the latest state of the plot without an update_display call.

martinRenou · 2021-09-21T08:00:38Z

Now using a post_execute cell hook (similar to what the inline backend does), this allows getting rid of the roundtrip to the client and not having to use the update_display (removing the flickering).

This seems to work nicely, though there is still a bit of polishing to do.

martinRenou · 2021-09-21T09:39:03Z

cc. @ianhi @SylvainCorlay @tacaswell

This seems to work nicely in JupyterLab now! It makes ipympl closer to the inline backend in behavior and implementation.

It will work in classic Jupyter Notebook when notebook gets released and widgetsnbextension gets released with Widgetsnbextension: throw error on failure to render jupyter-widgets/ipywidgets#3280.
It will work with nbconvert when HTMLExporter: Add a new output filter that excludes widgets if there is no state jupyter/nbconvert#1643 is released

We can merge this PR now and not wait for notebook and nbconvert to update.

martinRenou · 2021-09-21T09:47:11Z

SylvainCorlay · 2021-09-21T09:52:37Z

Let's get this in as it is a net improvement over the current situation, and tag a patch release of ipympl.

martinRenou · 2021-09-21T10:16:39Z

Let's make a minor release instead, as the API changes a tiny bit: it's not plt.figure() which makes the display anymore, so wrapping it inside an Output widget doesn't work anymore.

SylvainCorlay · 2021-09-21T10:17:44Z

Placing show in an output widget shoulds still work as expected though.

jklymak · 2021-09-21T12:53:52Z

Thanks for the persistence on this one! Sounds like it was tricky...

ianhi · 2021-09-22T17:41:16Z

ipympl/backend_nbagg.py

-        # Figure width in pixels
-        pwidth = (self.canvas.figure.get_figwidth() *
-                  self.canvas.figure.get_dpi())
-        # Scale size to match widget on HiDPI monitors.
-        if hasattr(self.canvas, 'device_pixel_ratio'):  # Matplotlib 3.5+
-            width = pwidth / self.canvas.device_pixel_ratio
-        else:
-            width = pwidth / self.canvas._dpi_ratio
-        data = "<img src='data:image/png;base64,{0}' width={1}/>"
-        data = data.format(b64encode(buf.getvalue()).decode('utf-8'), width)


@martinRenou why is it ok to remove this dpi scaling? It seems like we will now have imperfect behavior on hi dpi displays.

This might have been a mistake, we should open an issue to track this

I'll do it, actually. If I understand correctly, the PNG doesn't get saved at high res.

Note: we should not determine the dpi based on the browser because it could differ between clients.
It should be a fixed value.

ianhi · 2021-09-22T17:46:03Z

Let's make a minor release instead, as the API changes a tiny bit: it's not plt.figure() which makes the display anymore, so wrapping it inside an Output widget doesn't work anymore.

I can see this making #290 harder because now figures aren't shown until the end of cell execution? Or maybe that can be made to work with an explicit call to plt.show in a notebook cell?

SylvainCorlay · 2021-09-23T09:04:27Z

I can see this making #290 harder because now figures aren't shown until the end of cell execution? Or maybe that can be made to work with an explicit call to plt.show in a notebook cell?

Yes, I actually think that the interactive mode (plt.ion()) is somewhat intrinsically broken. I would prefer it to be off by default and explicit calls to plt.show() to be required...

nvaytet · 2022-06-29T14:37:29Z

Hi, unfortunately, I am still seeing this error after re-opening the notebook.

The console log from Chrome:

Versions:

jupyter_client            7.3.1              pyhd8ed1ab_0    conda-forge
jupyter_core              4.10.0           py39hf3d152e_0    conda-forge
jupyter_server            1.17.0             pyhd8ed1ab_0    conda-forge
jupyterlab                3.4.2              pyhd8ed1ab_0    conda-forge
jupyterlab_pygments       0.2.2              pyhd8ed1ab_0    conda-forge
jupyterlab_server         2.14.0             pyhd8ed1ab_0    conda-forge
jupyterlab_widgets        1.1.0              pyhd8ed1ab_0    conda-forge

matplotlib                3.5.2            py39hf3d152e_0    conda-forge
matplotlib-base           3.5.2            py39h700656a_0    conda-forge
matplotlib-inline         0.1.3              pyhd8ed1ab_0    conda-forge

ipydatawidgets            4.3.1.post1        pyhc268e32_0    conda-forge
ipykernel                 6.13.0           py39hef51801_0    conda-forge
ipympl                    0.9.1              pyhd8ed1ab_0    conda-forge
ipython                   8.4.0            py39hf3d152e_0    conda-forge
ipython_genutils          0.2.0                      py_1    conda-forge
ipywidgets                7.7.0              pyhd8ed1ab_0    conda-forge

Thanks for any help!

jklymak · 2022-06-29T14:39:46Z

@nvaytet, I'd open a new issue - but for what it is worth, I have the same problem as you....

SylvainCorlay and others added 2 commits September 14, 2021 16:21

Add text-html repr

eb94d22

Update display on first render

8fb40c7

martinRenou commented Sep 14, 2021

View reviewed changes

SylvainCorlay mentioned this pull request Sep 14, 2021

Add text-html repr #59

Closed

This was referenced Sep 14, 2021

Stop button #176

Closed

Embed image on save #294

Closed

Fix issue for latest ipywidgets

b5f42aa

martinRenou force-pushed the text-html branch from 865af81 to b5f42aa Compare September 14, 2021 16:12

martinRenou mentioned this pull request Sep 15, 2021

Recover from failure to render mimetype jupyter/notebook#6181

Merged

ianhi mentioned this pull request Sep 15, 2021

update binder env #344

Closed

SylvainCorlay marked this pull request as ready for review September 15, 2021 16:39

martinRenou mentioned this pull request Sep 16, 2021

HTMLExporter: Add a new output filter that excludes widgets if there is no state jupyter/nbconvert#1643

Merged

martinRenou marked this pull request as draft September 16, 2021 10:00

martinRenou commented Sep 16, 2021

View reviewed changes

ipympl/backend_nbagg.py Show resolved Hide resolved

martinRenou mentioned this pull request Sep 20, 2021

Feature: save additional mime bundles in notebook jupyter-widgets/ipywidgets#3107

Open

5 tasks

martinRenou added 3 commits September 20, 2021 17:03

Solution that works without update_display

a373f5c

Globally save the current dpi

fefe076

Dummy display

a589859

Use post_execute hook for showing figures

af699c2

martinRenou added 4 commits September 21, 2021 10:14

Iterate

bce06fa

Clean

113a15d

Cleaning

783419b

Update "docs"

bc39d8f

martinRenou marked this pull request as ready for review September 21, 2021 09:33

martinRenou requested a review from ianhi September 21, 2021 09:34

Review

bbfe34d

SylvainCorlay merged commit 4faf35f into matplotlib:master Sep 21, 2021

SylvainCorlay deleted the text-html branch September 21, 2021 12:49

martinRenou mentioned this pull request Sep 21, 2021

Images not saved in *.ipynb #16

Closed

ianhi reviewed Sep 22, 2021

View reviewed changes

ianhi mentioned this pull request Sep 22, 2021

Follow-up to #343 - Embedding Static Plots #359

Open

jklymak mentioned this pull request Sep 23, 2021

New static repr of matplotlib plots saved at low-res #361

Closed

dokempf mentioned this pull request Oct 22, 2021

Filter tuning app does not show hillshade in Sphinx documentation ssciwr/afwizard#65

Closed

ml8 mentioned this pull request Oct 28, 2021

Poll faculty for spring changes + improvements Rhodes-CS-Department/jupyterhub-deployment#30

Open

nvaytet mentioned this pull request Jun 29, 2022

Still getting "Model not found" error instead of static figure when saving and re-opening the notebook #473

Open

ianhi mentioned this pull request Feb 21, 2023

Plot output with ipywidgets formatting incorrectly #436

Open

trchudley mentioned this pull request Aug 19, 2023

Closing a figure from the notebook does not close the python figure #4

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement image/png repr #343

Implement image/png repr #343

martinRenou commented Sep 14, 2021 •

edited

github-actions bot commented Sep 14, 2021

martinRenou Sep 14, 2021

SylvainCorlay Sep 14, 2021

martinRenou Sep 14, 2021

martinRenou commented Sep 14, 2021

SylvainCorlay commented Sep 14, 2021

ianhi commented Sep 14, 2021

martinRenou commented Sep 15, 2021

martinRenou commented Sep 15, 2021

ianhi commented Sep 15, 2021

ianhi commented Sep 15, 2021

martinRenou commented Sep 15, 2021

martinRenou commented Sep 16, 2021

ianhi commented Sep 16, 2021

martinRenou commented Sep 16, 2021

martinRenou commented Sep 21, 2021

martinRenou commented Sep 21, 2021

martinRenou commented Sep 21, 2021

SylvainCorlay commented Sep 21, 2021

martinRenou commented Sep 21, 2021

SylvainCorlay commented Sep 21, 2021

jklymak commented Sep 21, 2021

ianhi Sep 22, 2021

martinRenou Sep 23, 2021

jklymak Sep 23, 2021

SylvainCorlay Sep 23, 2021

ianhi commented Sep 22, 2021

SylvainCorlay commented Sep 23, 2021

nvaytet commented Jun 29, 2022

jklymak commented Jun 29, 2022 •

edited

Implement image/png repr #343

Implement image/png repr #343

Conversation

martinRenou commented Sep 14, 2021 • edited

github-actions bot commented Sep 14, 2021

martinRenou Sep 14, 2021

Choose a reason for hiding this comment

SylvainCorlay Sep 14, 2021

Choose a reason for hiding this comment

martinRenou Sep 14, 2021

Choose a reason for hiding this comment

martinRenou commented Sep 14, 2021

SylvainCorlay commented Sep 14, 2021

ianhi commented Sep 14, 2021

martinRenou commented Sep 15, 2021

martinRenou commented Sep 15, 2021

ianhi commented Sep 15, 2021

ianhi commented Sep 15, 2021

martinRenou commented Sep 15, 2021

martinRenou commented Sep 16, 2021

ianhi commented Sep 16, 2021

martinRenou commented Sep 16, 2021

martinRenou commented Sep 21, 2021

martinRenou commented Sep 21, 2021

martinRenou commented Sep 21, 2021

SylvainCorlay commented Sep 21, 2021

martinRenou commented Sep 21, 2021

SylvainCorlay commented Sep 21, 2021

jklymak commented Sep 21, 2021

ianhi Sep 22, 2021

Choose a reason for hiding this comment

martinRenou Sep 23, 2021

Choose a reason for hiding this comment

jklymak Sep 23, 2021

Choose a reason for hiding this comment

SylvainCorlay Sep 23, 2021

Choose a reason for hiding this comment

ianhi commented Sep 22, 2021

SylvainCorlay commented Sep 23, 2021

nvaytet commented Jun 29, 2022

jklymak commented Jun 29, 2022 • edited

martinRenou commented Sep 14, 2021 •

edited

jklymak commented Jun 29, 2022 •

edited