search

mne-tools / mne-python

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

MNE Report: Add Events produces cut-off plots. Can we do better? #12835

Open sappelhoff opened 2 months ago

sappelhoff commented 2 months ago

For example, look at this screenshot, from a report where an Events section was added through report.add_events:

image

☝️ The plot is "cut off" in the legend box, both in terms of height and width.

Furthermore, the y-axis ticklabels are very tightly squashed together

If I want to get the non-cutoff version of this plot, then I cannot currently use report.add_events.

I need to replicate the plot myself using plot_events, setting a proper figure size, saving the plot using bbox_inches="tight", and then adding the file with add_image, which is a bit inconvenient.

Would it be reasonable to allow for a bit more customization on how much space report.add_events may reserve for the plot? So that I could also produce plots that are not cut-off with the easier add_events workflow?

larsoner commented 2 months ago

I think there are a couple of things we could do.

My first thought was to switch plot_events to use the constrained layout engine. It's almost always better about spacing. This won't fix the problem but might make it a little better, and it's something we should probably do anyway.

Next would be, for reports in add_events, I suggest we plot the events (probably starting from plot_events) but remove the legend, add the legend-less plot, and then separately add a legend maybe below the plot, ideally in some nice way in HTML. Reports are HTML, we should take advantage of that by using stuff like overflow scrolling for cases like this where we don't know if there will be one or hundreds of events.

sappelhoff commented 2 months ago

My first thought was to switch plot_events to use the constrained layout engine. It's almost always better about spacing. This won't fix the problem but might make it a little better, and it's something we should probably do anyway.

In the hope of a low hanging fruit, I just looked into that but found that we are already using "constrained layout". See:

https://github.com/mne-tools/mne-python/blob/f3a3ca4430e1d4b9c539e7949c946f4f83bdb43f/mne/viz/misc.py#L846

or did you have something else in mind?

larsoner commented 1 month ago

or did you have something else in mind?

Nope that's what I had in mind, just didn't look at the code :facepalm: