Remove average parameter from CSP and SPoC plotting methods
Describe the new feature or enhancement
Discussed in call with @wmvanvliet. Not so much a feature request but a feature un-request...
The CSP and SPoC classes of the decoding module have the plotting methods plot_filters() and plot_patterns() for visualising the filters and patterns of each component as a topomap. Internally, this plotting works by packaging the filters/patterns into an EvokedArray object and calling the plot_topomap() method.
Evoked.plot_topomap() has the average parameter for plotting data averaged over times, which is also supported by the plot_filters/patterns() methods of the CSP and SPoC classes, where here it functions to average over components. This has two issues:
- The docstring entry for the decoding classes is the same as in
Evoked.plot_topomap, so it refers to averaging over times: "The time window (in seconds) around a given time point to be used for averaging. For example, 0.2 would translate into a time window that starts 0.1 s before and ends 0.1 s after the given time point...". - It is questionable whether there should be the option to average over these independent components. They should really be handled separately and having the
averagemethod could give a user not so familiar with the methods the impression that this is an acceptable way to interpret the spatial filters/patterns.
Describe your proposed implementation
Remove the average parameter from the plot_filters() and plot_patterns() methods of mne.decoding.CSP and mne.decoding.SPoC classes.
Describe possible alternatives
If people are against removing this functionality, at the very least could update the docstrings to remove references to averaging over times, and instead to averaging over components which is what the argument is actually doing.
Additional context
No response
This is another example how the docdict does not make docstrings more consistent. I don't have an opinion regarding what to do about the averaging, but at the very least I'd add the correct docstring in plain text (no docdict entry since we'll hopefully be moving away from it).
This is another example how the docdict does not make docstrings more consistent
We've discussed this before elsewhere. The docdict does improve consistency but does not prevent mistakes/inaccuracies (like this one). As you note, we're trying to come up with a better system. Until we do that, can we refrain from requesting that every PR reduce docdict usage? The primary question here is whether to keep the parameter at all, or just change the param description
+1 to remove the param entirely.
Just wanted to follow up on this. Seems there is support from @mscheltienne and @wmvanvliet to remove the parameter. If others approve and I would open a PR, should it just be a deprecation warning for now?
+1 to remove. Should get a deprecation cycle