[BUG] Fix issue in decomposition plotting to specified axes

[tsbinns]

When playing around with customising the plotting of the decomposition filters and patterns, I realised the parameter to pass in axes for plotting the data to doesn't work as intended.

Here there is separate plotting for the seeds and targets, but axes expects just one set of plots. This means if you pass in the desired axes to plot to, the seeds will be plotted first, then the targets will be plotted and overwrite the seeds.

A solution is to pass in 2 sets of axes, one for the seeds and one for the targets. After that everything is handled by the existing topo plotting.

[tsbinns]

Updated the docstring and also added more info about the required number of axes when colourbar is plotted or not.

[tsbinns]

This was missing before #232 was merged. My bad.

[tsbinns]

Would also like to get opinions on how figure suptitle is handled.

By default, the seeds figure gets the title "Seeds", then the targets figure "Targets". However, if custom axes as passed corresponding to the same figure, the seed title is set first, which is then overwritten with the targets title.

This can be very easily fixed after the fact with a call to fig.suptitle() and the plotting functions do already have a very lengthy set of params, but it could be cleaner to have a title param of some sort. What do people think?

[drammock]

Would also like to get opinions on how figure suptitle is handled.

By default, the seeds figure gets the title "Seeds", then the targets figure "Targets". However, if custom axes as passed corresponding to the same figure, the seed title is set first, which is then overwritten with the targets title.

This can be very easily fixed after the fact with a call to fig.suptitle() and the plotting functions do already have a very lengthy set of params, but it could be cleaner to have a title param of some sort. What do people think?

In MNE-Python I've generally been moving away from including title params (because fig.suptitle is so easy to do), except in cases where the automatic title includes some informative / calculated value. For example, funcs that generate 3 topoplot figures (mag, grad, EEG) need the title to differentiate (it would be possible to infer which is which from the sensor layouts and the fact that grad is strictly positive... but it's a pain).

In this case, off the cuff I'd be inclined to auto-title each axes as "seed 0", "seed 1", ..., "target 0", "target 1", etc. and not include an automatic figure title. WDYT?

[tsbinns]

In MNE-Python I've generally been moving away from including title params (because fig.suptitle is so easy to do), except in cases where the automatic title includes some informative / calculated value. For example, funcs that generate 3 topoplot figures (mag, grad, EEG) need the title to differentiate (it would be possible to infer which is which from the sensor layouts and the fact that grad is strictly positive... but it's a pain).
In this case, off the cuff I'd be inclined to auto-title each axes as "seed 0", "seed 1", ..., "target 0", "target 1", etc. and not include an automatic figure title. WDYT?

Sure, that makes sense.

Just pushed some changes to remove the suptitle and added the seeds/targets name to the axes titles.
I also added a little more info in the docstring that a format specifier for the component number is required (this is expected by the topomap plotting).

Pretty happy with this:

[drammock]

This looks good. If I were to nitpick, I would say 2 things (both of which you can take or leave / use your own judgment):

1. The axes titles don't need underscores, just use spaces (and maybe parentheses?) e.g. "MIC 0 (seeds)"
2. It's a bit odd that the name_format specifier mixes f-string and %-string formatting. It could be something like f"{method} {idx:d} ({group})" (to yield what I suggest above in (1)). You'd need to update the docstring to explain what idx is of course.

[drammock]

Suggested change

	The string format for axes titles. If `None`, uses ``f"{method}%01d_{group}"``,
	The string format for axes titles. If ``None``, uses ``f"{method}%01d_{group}"``,

[tsbinns]

Elsewhere in the plotting docstrings the existing form is used which generates a link to the Python docs in the class params list, however for the method param docstrings this is just italicising the words (no links; also for True, str).

I didn't notice this difference before. Is this expected behaviour?
I don't mind changing to the double tick syntax where no links are generated, but if so I would go through and change elsewhere in the docstrings of this class for consistency.

[drammock]

sorry, I didn't spot this question before ticking auto-merge. Here's an answer anyway:

In the context of this sentence, None is Python code, in the sense of "if you pass None here's what you'll get" (substitute None for True or "auto" and it's clear). That's why I would double-backtick it. Now, in rST, single-backticks are annoying because they mean "try to resolve as a cross-reference using the default domain (which is :py:obj: for us I think), but if you can't resolve the xref, just treat like single-asterisks (AKA, italicize)". Our (probably not written down!) habit/tendency/policy is to never use single backticks (because of this ambiguity) and to always use explicit :func: :class: :meth: etc roles when we want an xref, always use double-backticks when we want code, and always use asterisks when we want italics.

[tsbinns]

The axes titles don't need underscores, just use spaces (and maybe parentheses?) e.g. "MIC 0 (seeds)"

Sure, have pushed this change.

[tsbinns]

It's a bit odd that the name_format specifier mixes f-string and %-string formatting. It could be something like f"{method} {idx:d} ({group})" (to yield what I suggest above in (1)). You'd need to update the docstring to explain what idx is of course.

The %-string formatting for the component numbers is used by plot_evoked_topomap() (there considered as time points), so this would need to be changed in the main MNE package.