ENH: Improve report plotting and animate_topomap#13905
Conversation
|
Ready for review/merge from my end! |
|
@drammock this one should be good to go, too |
| .. versionchanged:: 1.13 | ||
| The size of the obtained brain images (in pixels) now comes from the | ||
| ``size`` argument of ``stc_plot_kwargs``, which has a default | ||
| ``size=(450, 450)``. The ``report.img_max_res`` is now ignored. |
There was a problem hiding this comment.
seems weird to have a global report setting that doesn't apply globally. At a minimum this should be documented in the docstring for Report constructor, but I'd be curious to hear why it needs to be ignored now.
There was a problem hiding this comment.
I'll have to look... IIRC I did this because there are currently two ways of setting the size here (global val, and the val you pass to brain_kwargs) and to me it was surprising if I passed brain_kwargs(size=my_size) I would get an image that wasn't my_size. Like if we have img_max_res=(500, 500) and I pass brain_kwargs=dict(size=(800, 800)) should it generate the brain image at 800x800 then scale it down to img_max_res? I'll look into the actual behaviors here on main for this and other functions...
There was a problem hiding this comment.
I can see why that would be surprising. But if someone knew there was a max image size setting for the whole report, and it wasn't applying to the brain figures, that also seems surprising.
There was a problem hiding this comment.
Okay changed the behavior to respect report.image_max_width, and documented it!
I don't like how long it takes to generate reports, so I was trying to speed it up a bit. In doing so, I realized
animate_topomapwasn't working properly, and refactored it to reuseplot_evoked_topomapcode. This makes it more feature-complete, and I fixed its behavior (with bothblit=Trueandblit=False). I also made it so the report now hasbutterfly=Truefrom theanimate_topomap, which I think is nice. From this testing code:MWE
You can see it:
I expect some test breakages, will push fixes for those.