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

Stop button #176

Closed
wants to merge 1 commit into from
Closed

Conversation

martinRenou
Copy link
Member

@martinRenou martinRenou commented Feb 25, 2020

This should fix #150 and fix #16

It adds a "Stop interactions and embed" button which allows saving the plot as a static image in the Notebook. The output actually stays a widget, which has the last state of the plot in its model.

embed

Signed-off-by: martinRenou <martin.renou@gmail.com>
@martinRenou
Copy link
Member Author

cc. @tacaswell @SylvainCorlay

One idea would be to remove the interactive widget repr from the mimebundle repr when the Matplotlib figure is closed, and save the image repr instead.

Not sure I have all the implementation details in mind, but I feel like that would be an elegant way of solving this issue.

@xeliba
Copy link

xeliba commented May 19, 2020

This is a really important feature.

After "closing" , all plots should be simple PNG files in the Notebook. That enables us to export the whole notebook as html, pdf etc. Is this workflow already possible with this PR? What is still missing?

(By the way, with the matplotlib 'notebook' backend this is already possible for normal 'Jupyter' notebooks. Not for 'JupyterLab', yet.)

@martinRenou
Copy link
Member Author

Is this workflow already possible with this PR?

This PR is not really fixing this issue properly.
It only saves the image as part of the widget model. The problem is that the widget model is not always saved in the Notebook, so you might not be able to retrieve the image.

What is still missing?

I think we should take a different approach than what this PR is doing. The approach would be the one I tried to explain on the comment on top of yours:

When the close button is pressed, or when the Matplotlib figure is closed using the Python API, we should actually remove the Widget and replace it with a plain image in the cell output.

@xeliba
Copy link

xeliba commented May 19, 2020

Recently, I tried to obtain the PNG using the renderer of the 'widget' backend obtained with 'fig.canvas.get_renderer()' . But it only returned a buffer with all pixels of the same color, i.e. without the actual plot. Don't know what I have to call in advance. 'fig.canvas.draw()' does not help, which is little bit confusing.

@ianhi
Copy link
Collaborator

ianhi commented Jul 10, 2020

When the close button is pressed, or when the Matplotlib figure is closed using the Python API, we should actually remove the Widget and replace it with a plain image in the cell output.

Big fan of this feature existing. I'm willing to help - but any advice on what the appropriate way to accomplish this is would be great. Is this PR: #59 the approach to take?

@ianhi
Copy link
Collaborator

ianhi commented Jul 10, 2020

Looking at it further I do think that #59 shows gives the "implementation details"

My understanding is that the Widget class defines _repr_mime_bundle which will normally implant a view of widget into the notebook. So canvas should override this function with some logic that looks like this:

if not self.closed:
    Super()._repr_mimebundle()
else:
    "<img src='data:image/png;base64,{......}'/>"

@ianhi
Copy link
Collaborator

ianhi commented Jul 10, 2020

In https://ipython.readthedocs.io/en/stable/api/generated/IPython.display.html#functions it says that:

A single object can declare some or all of these representations; all are handled by IPython’s display system.

referring to this:
image

but if you define multiple of them then it's not clear how to signal which one should take precedence.

@martinRenou
Copy link
Member Author

if you define multiple of them then it's not clear how to signal which one should take precedence

I think that is actually the problem with the PR you pointed to. If I'm correct, it's the UI that chooses which representation to display. I think the widgets representation would always take precedence over the HTML one.

I'm willing to help - but any advice on what the appropriate way to accomplish this is would be great. Is this PR: #59 the approach to take?

Honestly, I really don't know. I would love to see a feature like this. And I think it's a big blocker for people to switch from the inline back-end to the widget one.

If you want to implement this you are very welcome to do so! :)

I am wondering if we could use a smart IPython.display.update_display call to update the bundle and remove the widget representation when the figure is closed.

@ianhi
Copy link
Collaborator

ianhi commented Jul 16, 2020

Some issues that look relevant:
jupyter-widgets/ipywidgets#2280
jupyter-widgets/ipywidgets#2682

and for completeness: https://gitter.im/jupyter-widgets/Lobby?at=5f0ea2e30d37916fda770757

@martinRenou
Copy link
Member Author

Reading jupyter-widgets/ipywidgets#2682 I feel like we have our solution! The person here seem to be able to use IPython.display.update_display for updating the bundle, I actually think this would be the way to go!

We would not have the issue the person mentions, simply because we would remove the widget representation so no need for re-rendering the widget, instead the UI would re-render the plot as a static image. This is what we want I guess, what do you think? :)

So I guess we can reuse the close button here, but this close button would send a figure "close" request to the Matplotlib API.
Then we need to override the closing behavior on the canvas (I hope this is possible), which would update the display bundle.

@ianhi
Copy link
Collaborator

ianhi commented Jul 16, 2020

Would these lines

def closer(event):
Gcf.destroy(num)
also destroy the widget object?

@martinRenou
Copy link
Member Author

To be totally honest with you I am not really comfortable with the Python code in this repository, it was copied from the nbagg back-end if I'm correct.

What if you override the close method on the Canvas and put a print statement in it, do you see it called when closing the figure?

@ianhi
Copy link
Collaborator

ianhi commented Jul 16, 2020

To be totally honest with you I am not really comfortable with the Python code in this repository,

Sadly that makes two of us

It looks like the nbagg backend (only works in notebook not in jlab) has figured out how to embed the plot as an image:
nbagg-embed

with relevant js code here: https://github.com/matplotlib/matplotlib/blob/81a560ccff1b1e369641a8687fe132d202037d4a/lib/matplotlib/backends/web_backend/js/nbagg_mpl.js#L162-L168

Button calls a js method (https://github.com/matplotlib/matplotlib/blob/81a560ccff1b1e369641a8687fe132d202037d4a/lib/matplotlib/backends/web_backend/js/nbagg_mpl.js#L53-L66) that just sets the parent node innerHTML to be an img tag

So that works but it seems that the discussed mime_bundle is a more principled approach

@tacaswell
Copy link
Member

If you do display(fig) you will get a static png (due to some other repr hooks registered by IPython).

@tacaswell
Copy link
Member

See the discussion in matplotlib/matplotlib#17891 for more details about how Matplotlib currently interacts with the display system.

@ianhi
Copy link
Collaborator

ianhi commented Aug 13, 2020

I spent some time messing around with rewriting the rendermime method and trying to use update_display but couldn't get anything working. I think this is due to widgets not playing nicely with update_display jupyter-widgets/ipywidgets#1180

@martinRenou
Copy link
Member Author

Is there a Javascript error when trying to update the display?

@jklymak
Copy link
Member

jklymak commented Sep 8, 2020

This does not close #16 in my opinion. Having to remember to click each figure is still a pretty big regression from nbagg. Not as bad as no way to save the image, but close.

@martinRenou
Copy link
Member Author

Maybe I am completely missing something here, but I don't see an automatic way of updating the display for replacing the widget by a static image. So either the user will have to click the button, or they will need to programmatically close the figure.

@martinRenou
Copy link
Member Author

Anyway, this PR has a draft status because it simply does not work. We'll need to find a better implementation.

@ianhi
Copy link
Collaborator

ianhi commented Sep 8, 2020

but I don't see an automatic way of updating the display for replacing the widget by a static image.

  1. I think the ipywidgets issue with update_display being solved would be very helpful
  2. We can maybe emulate the nbagg behavior by adding an callback to the comm being closed. On the comm closing this.comm.on_close(somemethod.bind(this)) we can modify the html of the parent element to be an img tag?

@martinRenou
Copy link
Member Author

Not sure listening to the comm being closed would help unfortunately, but I might be wrong. I would suspect the comm is not closed if you save the Notebook, close the tab and kill the Notebook server.

@jklymak
Copy link
Member

jklymak commented Sep 8, 2020

So either the user will have to click the button, or they will need to programmatically close the figure.

For nbagg, the image is just saved as part of the static representation, without user interaction. I have no idea if that is just fundamentally impossible for widgets or not. However, if the goal is to make a notebook available for posterity, having the static png in the notebook is a fundamental requirement. If you don't have it, then the "widget" is just a transient interactive environment. Which is fine, but I don't think ipympl should be touted as a replacement for nbagg without this ability.

@martinRenou
Copy link
Member Author

What makes the difference between nbagg and ipympl is the following:

  • nbagg saves in the cell output an application/javascript mimetype representation that contains the code needed for when the plot is interactive, and a text/html mimetype representation for when the plot is static.
  • ipympl saves in the cell output a custom ipywidgets mimetype representation which contains the widget model (and the widget manager is responsible for rendering the widget given this widget model).

We could save a text/html mimetype in the cell output, but that would not fix this issue, because the front-end (Jupyter Notebook or JupyterLab) will always render the widget representation if it's available, it has precedence over the static HTML representation. We actually cannot do anything about this in ipympl. That would be something to fix upstream I guess.

One thing we were trying to look at with @ianhi was to remove the widget representation and replace it with a static text/html representation, so either when the figure is closed programmatically or when pressing the close button. This might be doable using the IPython's update_display, but this does not seem to work unfortunately.

I don't think ipympl should be touted as a replacement for nbagg without this ability.

I totally agree with this. Although to be fair, ipympl has the upside of working in JupyterLab and Voila, which is not the case for nbagg.

I would love to see this issue correctly fixed, any idea is very welcome.

@martinRenou
Copy link
Member Author

martinRenou commented Sep 8, 2020

What we would need would be a fallback to the static HTML representation when the widget manager is not able to render the widget. That might be the perfect fix. But this is not anymore an ipympl issue then, it's an ipywidgets issue I guess, or even a Jupyter Notebook/JupyterLab issue.

@martinRenou
Copy link
Member Author

Closing as outdated. We will try another approach for saving a static representation of the plot #343

@martinRenou martinRenou deleted the stop_button branch September 14, 2021 15:43
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

Successfully merging this pull request may close these issues.

embed and close button Images not saved in *.ipynb
5 participants