Add axes argument for montage plotting#11404
Conversation
Allows integrating montage plots into other plots.
|
Hello! 👋 Thanks for opening your first pull request here! ❤️ We will try to get back to you soon. 🚴🏽♂️ |
| @verbose | ||
| def plot_montage(montage, scale_factor=20, show_names=True, kind='topomap', | ||
| show=True, sphere=None, verbose=None): | ||
| show=True, sphere=None, verbose=None, axes=None): |
There was a problem hiding this comment.
| show=True, sphere=None, verbose=None, axes=None): | |
| show=True, sphere=None, axes=None, *, verbose=None): |
verbose must always come last. You'll need to update the order of docstring parameter entries too.
There was a problem hiding this comment.
I was worried about backward compatibility with this. Making it a kwarg-only attribute sounds great. So I'll also implement that change of making verbose a kwarg in these functions?
There was a problem hiding this comment.
yes. We're sort of in transition to making wider use of kwarg-only parameters. verbose is one that should always be kwarg-only; others are sort of case-by-case and for now are mostly being done in functions/methods with really long parameter lists.
| @copy_function_doc_to_method_doc(plot_montage) | ||
| def plot(self, scale_factor=20, show_names=True, kind='topomap', show=True, | ||
| sphere=None, verbose=None): | ||
| sphere=None, verbose=None, axes=None): |
There was a problem hiding this comment.
| sphere=None, verbose=None, axes=None): | |
| sphere=None, axes=None. *, verbose=None): |
ditto
| axes : instance of Axes | instance of Axes3D | None | ||
| Axes to draw the sensors to. If ``kind='3d'``, axes must be an instance | ||
| of Axes3D. If None (default), a new axes will be created. | ||
|
|
||
| .. versionadded:: 1.4.0 |
There was a problem hiding this comment.
Ideally this would become
| axes : instance of Axes | instance of Axes3D | None | |
| Axes to draw the sensors to. If ``kind='3d'``, axes must be an instance | |
| of Axes3D. If None (default), a new axes will be created. | |
| .. versionadded:: 1.4.0 | |
| %(axes_plot_sensors)s | |
| .. versionadded:: 1.4.0 |
and then a similar change here:
Lines 939 to 943 in ea88c55
... and then the duplicated docstring gets added to the docdict in between these two entries:
Lines 283 to 285 in ea88c55
If that sounds doable, please give it a try; if not LMK and I can push a commit that does this.
Allows integrating montage plots into other plots.