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

Implement generateMimebundle #300

Closed
wants to merge 2 commits into from

Conversation

martinRenou
Copy link
Member

@martinRenou martinRenou mentioned this pull request Feb 11, 2021
@martinRenou martinRenou marked this pull request as draft February 11, 2021 16:11
@ianhi
Copy link
Collaborator

ianhi commented Feb 11, 2021

Something I meant to comment on #294: Will this stop widget interaction on a save event? Ideally the interaction will stay open on saving. I know that I regularly save as a matter of habit so that would be a pretty big disruption.

Is it the case that this mimebundle will only be displayed if the model state is not available?

@ianhi
Copy link
Collaborator

ianhi commented Feb 11, 2021

also: hoooooooooooooooorayyyyyyyyyyyyy - I'm so happy y'all are making progress on this

@@ -240,6 +240,13 @@ export class MPLCanvasModel extends widgets.DOMWidgetModel {
}
}

generateMimeBundle() {
return Promise.resolve({
'text/html': `<img src="${this.offscreen_canvas.toDataURL('image/jpeg', 1.0)}"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IDK actually, it's the first idea that came to my mind. We can do PNG then :)

@martinRenou
Copy link
Member Author

martinRenou commented Feb 11, 2021

Something I meant to comment on #294: Will this stop widget interaction on a save event? Ideally the interaction will stay open on saving. I know that I regularly save as a matter of habit so that would be a pretty big disruption.

It seems that saving the Notebook does not change the content of the Notebook when looking at it. The widget stays there and doesn't get re-rendered.

Not sure I exactly understand how this works.

@martinRenou
Copy link
Member Author

martinRenou commented Feb 12, 2021

@ianhi the fact that Maarten 's PR does not overwrite the widget state from the mimebundle will be a blocker in the case of ipympl.

Given the current implementation of ipympl, even if we have a widget model, we are not able to recover the plot image (because it's not part of the model).

Also, even if we made the plot image part of the model, when opening a Notebook saved by someone else, the widget will not be interactive anyway, because the new back-end is not aware of the data it should plot if we interact with the buttons.

I would be in favor of adding a custom save hook in ipympl for Jupyter Notebook, that would remove the widget repr from the bundle, and only save the static image. What do you think?

@sin-mike
Copy link

@ianhi the fact that Maarten 's PR does not overwrite the widget state from the mimebundle will be a blocker in the case of ipympl.

Given the current implementation of ipympl, even if we have a widget model, we are not able to recover the plot image (because it's not part of the model).

Also, even if we made the plot image part of the model, when opening a Notebook saved by someone else, the widget will not be interactive anyway, because the new back-end is not aware of the data it should plot if we interact with the buttons.

I would be in favor of adding a custom save hook in ipympl for Jupyter Notebook, that would remove the widget repr from the bundle, and only save the static image. What do you think?

Dear @martinRenou , is there any resolution to the issue yet? I'm really looking forward to the working raster previews saved in NBs. If not, what would you suggest as a temporary workaround for now?

Sincerely,
Mike

@martinRenou
Copy link
Member Author

martinRenou commented Sep 13, 2021

cc. @ianhi

Changing jupyter-widgets/ipywidgets#3107 so that it adds the possibility to completely overwrite the mimebundle gives the most wanted result:

ipympl_sav

Will push the commits soon-ish.

** Edit: ** But we need a similar thing for JupyterLab

@martinRenou
Copy link
Member Author

New update

ipympl_save

@martinRenou
Copy link
Member Author

If not, what would you suggest as a temporary workaround for now?

@sin-mike there is no workaround possible as far as I know. But I am hopeful we can fix this soonish.

@ianhi
Copy link
Collaborator

ianhi commented Sep 14, 2021

Dear @martinRenou , is there any resolution to the issue yet? I'm really looking forward to the working raster previews saved in NBs. If not, what would you suggest as a temporary workaround for now?

@sin-mike it's very much not ideal but if you get a figure to where you want it then there are two options to embed a static version in the notebook:

  1. display(fig)
  2. Screenshot/save and paste into markdown cell (if screenshot goes to clipboard you can directly paste into markdown cell)

@martinRenou looking great. I imagine most of the work for this will live on the ipywidgets pr?

@martinRenou
Copy link
Member Author

@martinRenou looking great. I imagine most of the work for this will live on the ipywidgets pr?

Yeah. After some discussions with @SylvainCorlay we agreed that this was too much of a hack. We're trying a cleaner approach.

@SylvainCorlay
Copy link
Member

Closing!

@martinRenou martinRenou deleted the generateMimeBundle branch September 14, 2021 15:42
@sin-mike
Copy link

1. display(fig)

Wow, it's really a quick way to rasterize plots. Thanks!

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.

4 participants