mne-tools / mne-python

MNE: Magnetoencephalography (MEG) and Electroencephalography (EEG) in Python
https://mne.tools
BSD 3-Clause "New" or "Revised" License
2.7k stars 1.31k forks source link

Make _plot_image public #5952

Open jona-sassenhagen opened 5 years ago

jona-sassenhagen commented 5 years ago

evoked and tfr image plots have some fairly elaborate code to plot images with mask and contour. We could expose the (shared) underlying code publicly, greatly simplifying plotting of e.g. statistically thresholded GAT matrices.

larsoner commented 5 years ago

Okay with me

agramfort commented 5 years ago

what would be the signature of the public funciton?

jona-sassenhagen commented 5 years ago

The private function is here:

https://github.com/mne-tools/mne-python/blob/master/mne/viz/utils.py#L2556

Current call signature:

def _plot_masked_image(ax, data, times, mask=None, picks=None, yvals=None,
                       cmap="RdBu_r", vmin=None, vmax=None, ylim=None,
                       mask_style="both", mask_alpha=.25, mask_cmap="Greys",
                       yscale="linear"):

Probably ax should become a default kwarg ... data is mandatory ... I'd switch times and yvals for xvals and yvals and make both default to None ... maybe add a title, x/ylabel, cbar. So:

def plot_masked_image(data, ax=None, xvals=None, yvals=None, mask=None, picks=None, 
                       cmap="RdBu_r", vmin=None, vmax=None, ylim=None,
                       mask_style="both", mask_alpha=.25, mask_cmap="Greys",
                       yscale="linear", title=None, xlabel=None, ylabel=None, cbar=False):

@cbrnr @mmagnuski

mmagnuski commented 5 years ago

I'm okay with that. Would it be better to call it plot_image, not plot_masked_image as the mask is optional? Or maybe plot_image_with_optional_mask_if_you_want_it ;)

agramfort commented 5 years ago

ok but please demo this in an existing example

jona-sassenhagen commented 5 years ago

I will! Thanks guys.