FIX save_image() dpi

[christianbrodbeck]

When setting different DPI in rcParams saved images are wrongly scaled (see below). I suspect it's because unlike matplotlib's imsave, PySurfer does not use dpi when saving the image. This PR fixes this.

[agramfort]

ok for you @mwaskom ?

[codecov-io]

Codecov Report

❗ No coverage uploaded for pull request base (master@baca853). Click here to learn what that means.
The diff coverage is 50%.

@@            Coverage Diff            @@
##             master     #241   +/-   ##
=========================================
  Coverage          ?   64.86%           
=========================================
  Files             ?        7           
  Lines             ?     2479           
  Branches          ?      494           
=========================================
  Hits              ?     1608           
  Misses            ?      688           
  Partials          ?      183

[mwaskom]

I don't think I fully understand the failure mode here or how the PR fixes it? Could you code up a simple example to demonstrate it, ideally skipping PySurfer and just plotting a random image?

[larsoner]

I think we either need to pass dpi=100 to both the Figure constructor and in savefig, or in neither place (the latter is implemented here, and seems cleaner).

IIUC currently we set dpi in one place, and always to 100, so in the other place it presumably takes the DPI value from plt.rcParams. So in theory doing a plt.rc_context with a wildly different DPI followed by a brain.save_image(...) should show something like what @christianbrodbeck posted at the top.

[mwaskom]

There are two different matplotlib rcParams that set the dpi (figure.dpi and savefig.dpi) ... maybe that's the issue?

I think we should give users a way to set the DPI of their figure in a way that doesn't involve matplotlib rcParams. It won't at all be obvious why those would be relevant, and when making figures for a paper, you'll generally want a higher DPI than the matplotlib default of 72.

[larsoner]

Fine by me to have dpi kwarg to savefig to set the output DPI

[christianbrodbeck]

Yeah I can update the PR to add it as a parameter, I just did the least amount of change version that worked first :) But as a parameter it would only be marginally helpful because it does not actually change the number of pixels in the saved image, it just stores DPI as meta-information. I assume that’s why it was not exposed previously.

…

On Jun 4, 2018, at 10:45 AM, Eric Larson ***@***.***> wrote: Fine by me to have dpi kwarg to savefig to set the output DPI — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[mwaskom]

I guess that's a good point ... if we're limited by the DPI of the mayavi screenshot bitmap maybe it doesn't make sense to give the impression you could boost that further when saving an image file.

[larsoner]

True, and advanced users can still set the image metadata with plt.rc_context if they want

[larsoner]

Thanks @christianbrodbeck