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

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

Open
ianhi opened this issue Sep 22, 2021 · 11 comments
Open

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

ianhi opened this issue Sep 22, 2021 · 11 comments

Comments

@ianhi
Copy link
Collaborator

ianhi commented Sep 22, 2021

#343 was a huge step forward for saving the state of plots in notebooks. But it doesn't truly reproduce the state of the notebook that the author was looking at for these two cases:

  1. If widget state is saved then only a blank figure renders (Implement image/png repr #343 (comment))
  2. Any updates to the figure after the initial render are not saved in the notebook (Implement image/png repr #343 (comment))

I think part 2 is very important to be able to tell a story using a notebook. e.g. If I make some plots with a slider, get them just so and then run nbconvert I don't think I would faithfully reproduce the state of the notebook as I was looking at it.

Part 1 should be solvable if we include the PNG as part of the widget model

Part 2 I think a reasonable tradeoff would be to only update the static plot whenever the notebook is saved. This would require the frontend of ipywidgets (jupyterlab-manager, and widgetsnbextension) to listen to notebook save signals and pass them to the ipympl frontend, which would in turn let the backend know to update the mimebundle.

@martinRenou how hard do you think the second one (saving) would be to fix? To me that feels like a very important thing to hit.

@jklymak
Copy link
Member

jklymak commented Sep 22, 2021

I don't think classic notebook widgets saved the widget state either? From the point of view of reproducibility, I would argue that having the state of the figure set by sliders is suboptimal. Slide away to explore the data, but "publish" with hard-coded values.

@martinRenou
Copy link
Member

martinRenou commented Sep 23, 2021

  1. If widget state is saved then only a blank figure renders

I was planning on working on this next.

I guess the best approach for this is to save the image in the widget model but to not sync it with the JavaScript model, this way we don't send the data twice to the front-end, and animations are still decently fast.

  1. Any updates to the figure after the initial render are not saved in the notebook

Actually, whether or not we want to fix this is debatable. Do we want to save the state of the plot after interactions? Or do we want to save the plot that the Notebook generates when being executed? For the sake of reproducibility the second approach might be better. (edit, sorry @jklymak I see I'm repeating what you said, it's morning 😅 )

If we want to fix this, IPython's update_display might be our best shot. If we can get rid of the flickering this introduces. But it would make ipympl slower.

@SylvainCorlay
Copy link
Member

Any updates to the figure after the initial render are not saved in the notebook

Actually, whether or not we want to fix this is debatable. Do we want to save the state of the plot after interactions? Or do we want to save the plot that the Notebook generates when being executed? For the sake of reproducibility the second approach might be better. (edit, sorry @jklymak I see I'm repeating what you said, it's morning sweat_smile )

If we want to fix this, IPython's update_display might be our best shot. If we can get rid of the flickering this introduces. But it would make ipympl slower.

We initially did use the display with a display id (essentially what martin describes wrt update_display) and thought that the current solution was preferable.

If we updated the static rendering of previous cells like this, the resulting notebook including only the static plots would be very different from what you would obtain with e.g. the inline backend, as it would be a snapshot of the widget backend. Someone looking at the resulting notebook may not understand why an output of a first cell would not match what is done in the first cell.

If widget state is saved then only a blank figure renders (Implement image/png repr #343 (comment))

That should be resolved in a later PR.

@ianhi
Copy link
Collaborator Author

ianhi commented Sep 27, 2021

From the point of view of reproducibility, I would argue that having the state of the figure set by sliders is suboptimal

Any updates to the figure after the initial render are not saved in the notebook
Actually, whether or not we want to fix this is debatable. Do we want to save the state of the plot after interactions? Or do we want to save the plot that the Notebook generates when being executed? For the sake of reproducibility the second approach might be better. (edit, sorry @jklymak I see I'm repeating what you said, it's morning sweat_smile )

Someone looking at the resulting notebook may not understand why an output of a first cell would not match what is done in the first cell.

I definitely agree that for reproducibility the notebook author should not be using sliders to control the plots. But it seems to me that that decision should be left up to the notebook author. While looking at a saved notebook and seeing a cell output not matching up perfectly with it's code is surprising behavior (also possible already with out of order execution) I'd contend it is more surprising (and potentially damaging) to save a notebook and then having it look different when you (or someone else) opens it next. For example a student turning in a homework after having zoomed in on a region of an image, or similarly sending something to a colleague and asking what they think of a certain plot. In those case of course the authors should have made things fully reproducible, but I'd wager that people often don't do that.

Though, perhaps more importantly only saving the first draw of the figure is inconsistent with the behavior of the other notebook backends.

nbagg
Static image is as it was when the notebook was last saved.
Peek 2021-09-23 12-35

inline
In all cases the saved notebook is as the author was looking at it (even when using interact)

inline-persistence

ipympl with saving widget state
If we make it so that the widget also restores an image when it's full state is saved then it will be restoring the plot as last drawn.


(To be clear I definitely think as it stands this is already a great improvement :) )

@jklymak
Copy link
Member

jklymak commented Sep 27, 2021

Ah I see. I agree that if you add something in a different cell to an existing figure, it should also be saved.

@asteppke
Copy link

asteppke commented Sep 30, 2021

Could you add to the possible functions of a follow up of the initial feature that the export should work as well?

Currently when exporting the png version in Jupyterlab of the figure is not always included. A quick test shows:

  • HTML export
  • LaTeX export
  • Markdown
  • PDF (via LaTeX backend)
  • Reveal.js
  • WebPDF (via Chromium backend)

@martinRenou
Copy link
Member

martinRenou commented Oct 7, 2021

@ianhi your first point should be fixed by #369

@martinRenou
Copy link
Member

Partially fixed by #376

@martinRenou
Copy link
Member

martinRenou commented Dec 14, 2021

Some more bug fixes on the pdf export #404

And on the widget state export: #406

@astrojuanlu
Copy link

After gh-376, gh-404, and gh-406, I'm wondering what's missing?

@asteppke
Copy link

With the recent 0.9.1 when exporting the png version in Jupyterlab of the figure is included:

  • HTML export
  • LaTeX export
  • Markdown
  • PDF (via LaTeX backend)
  • Reveal.js
  • WebPDF (via Chromium backend, here nbconvert timed out, so probably not ipympl related)

What is not yet solved is the second part, i.e. when zooming or panning the state of the figure does not change in the saved static png. While maybe not essential this is something a user might assume when, for example, focusing on a certain region to emphasize something.

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

6 participants