ipympl icon indicating copy to clipboard operation
ipympl copied to clipboard

Save button should save in rcparams format, not "png"

Open thomasaarholt opened this issue 5 years ago • 10 comments

@tfblum noticed that the save button always saves images in the png format. In plt.rcParams['savefig.format'], one can specify the default saving format of plt.savefig, and it would be nice if the save button respected that.

That said, I think maybe the on-screen generated image already is a png? There are two references to "png" in the codebase: nbagg-backend, handle_save and handle_binary

thomasaarholt avatar Sep 18 '19 07:09 thomasaarholt

From #234 it's also seems that other savefig rcparams are not going to be respected. I guess the reason figure saving is structured the way it is right now is because we need javascript in order to open a download prompt. I see two potential solutions:

  1. Button pressed -> open jupyter dialog for savefig directory -> plt.savefig

  2. Use js to download the file but use plt.savefig to generate the buffer first.

    1. js On save button being pressed send a message to python side
    2. py buf = Bytes(); plt.savefig(buf')
    3. py self.send({'name':'saving-figure'}, [buf])
    4. js on:msg use current saving infrastructure.

ianhi avatar Jul 24 '20 19:07 ianhi

Hi - running into the same limitation. Are there any concrete plans to move one of the suggested solutions forward?

jkochNU avatar Mar 26 '22 00:03 jkochNU

@jkochNU I think this issue is still looking for a champion to push it forward. Could be you :)

If anyone is interested in working on this feel free to ask questions here and i can try to help

ianhi avatar Mar 26 '22 03:03 ianhi

@ianhi Happy to try, though this may turn out to be above my level of competence. I can navigate the py side, but am less familiar with the js side. Which js file is the one involved in (i) and (iv)?

jkochNU avatar Mar 26 '22 14:03 jkochNU

@jkochNU awesome! Dev installation instructions here: https://github.com/matplotlib/ipympl#for-a-development-installation-requires-nodejs

Here are some more concrete steps to solve this

Fyi: Saving starts when a user clicks the download button which sends a message to python that originates here: https://github.com/matplotlib/ipympl/blob/44a4ff246ec2fe0ca14238d089246aa9c8c6b270/src/toolbar_widget.ts#L157-L160

the toolbar message handling code just forwards all message handling to the canvas so that's where the work in python will go.

Python:

Add a new branch to the _handle_message: https://github.com/matplotlib/ipympl/blob/44a4ff246ec2fe0ca14238d089246aa9c8c6b270/ipympl/backend_nbagg.py#L261-L279

should check for content['type']=='toolbar_message' and content['name']=='save'

Then call a new method on the canvas object (named something like: _send_savefig_img which should call savefig into a bytes object - you can copy this implementation:

https://github.com/matplotlib/ipympl/blob/44a4ff246ec2fe0ca14238d089246aa9c8c6b270/ipympl/backend_nbagg.py#L335-L336 then send that to frontend with something like this:

self.send({'data': '{"type": "save"}'}, buffers=[data])

note: we may also need to include the filetype in this message. You can add it as a new key to the data dict - that is, as a sibling to the type key.

Typescript:

I think the only changes necessary will be contained to this function: https://github.com/matplotlib/ipympl/blob/44a4ff246ec2fe0ca14238d089246aa9c8c6b270/src/mpl_widget.ts#L151-L158

You will need to change the arguments of the function to be the same as the handle_binary function (msg: any, dataviews: any) and can copy or (pull into a function) the blob extraction code from handle_binary, chanigng the mimetype as appropriate. then modify the actual downloading (setting href and calling click) to look more like this SO answer: https://stackoverflow.com/a/23451803/835607

ianhi avatar Mar 27 '22 01:03 ianhi

@ianhi That's incredible. Thank you so much for the time and effort providing such detailed instructions. My turn to try my best implementing it.

jkochNU avatar Mar 27 '22 03:03 jkochNU

Before getting too deep it may be worth considering if this is consistent with the other matplotlib backends: https://gitter.im/matplotlib/matplotlib?at=623fb90e3ae95a1ec1ba9692

ianhi avatar Mar 27 '22 13:03 ianhi

Even if it's not, it's clearly desirable to more than one person - so I could also see either adding a new toolbar button, or making this a toggleable option.

ianhi avatar Mar 27 '22 13:03 ianhi

@ianhi I have a solution with a new toolbar button ready. However, I have not done extensive testing (this needs to work on multiple platforms, multiple Python versions, multiple browsers that you all support). Is there a workflow for this that I need to follow before posting a pull request?

jkochNU avatar Mar 27 '22 20:03 jkochNU

I have a solution with a new toolbar button ready

:tada:

Is there a workflow for this that I need to follow before posting a pull request?

nope. Just push changes to a new branch on your fork and open a PR and we'll take a look

ianhi avatar Mar 27 '22 22:03 ianhi