Scale points and labels in montage plot

[cbrnr]

Previously, the scale_factor argument only scaled the points in montage.plot(). This PR changes the behavior to also scale the channel labels.

In addition, I've changed the scale_factor so that 1 corresponds to the default size, values less than 1 scale down, and values greater than 1 scale up. I've also decreased the default point size for kind="topo" a little bit.

I've tested both kind="topo" and kind="3d", I think they both look good.

An alternative to scaling both dots and labels would be to keep the current behavior of scale_factor and add a new parameter font_size, which would allow users to change both properties independently.

[cbrnr]

import mne
mne.channels.make_standard_montage("biosemi64").plot(kind="topo", scale_factor=1)

Default old

Default new

scale_factor=0.75 new

scale_factor=1.5 new

scale_factor=80 old

scale_factor=0.5 and kind="3d" new

[cbrnr]

BTW, ideally I'd want to change scaling_factor to just scale. Do you think this is worth it?

[larsoner]

BTW, ideally I'd want to change scaling_factor to just scale. Do you think this is worth it?

One problem with the current PR is that it's a pretty big change for people who used this previously. We do often change viz functions but it's usually fairly subtle, but in this case a scale_factor=40 in main probably looks okay but will look wild (40x the normal size I think?) on this PR.

One way out of this is to:

1. leave scale_factor as it is and just document that really just sets the marker_size property of the collection essentially.
2. add a new scale=1. that does what this PR does, and scales the marker_size, text objects, etc. by that additional amount.

That stays 100% backward compatible but allows adding the new param and behavior with a better name.

[drammock]

One way out of this is to:

1. leave scale_factor as it is and just document that really just sets the marker_size property of the collection essentially.

2. add a new scale=1. that does what this PR does, and scales the marker_size, text objects, etc. by that additional amount.

I would add:

3. do a normal deprecation cycle on scale_factor and in the depr message encourage folks to use scale instead. If folks really need to control marker size and font size independently, we can (later) make it easy to get the handles of the various artists within the fig.mne namespace perhaps.

[cbrnr]

The functions have the following signature:

def plot_montage(
    montage,
    scale_factor=20,
    show_names=True,
    kind="topomap",
    show=True,
    sphere=None,
    *,
    axes=None,
    verbose=None,
):

Where would you like me to insert the new scale parameter? Ideally, it would be the second parameter and completely replace the current scale_factor after deprecation, but I'm not sure how flexible we are in changing parameter positions. We already use kw-only args, but only for the last two parameters. If possible, I'd move the * to right after montage.

[larsoner]

So far we've been willing to insert * at "reasonable places" knowing that users who need to adapt code 1) will get a reasonable error message, 2) those changes are backward- and forward-compatible, and 3) any changes they need to make are pretty straightforward (i.e., switch to keyword args). In this case we'd have to move the * to the second position which is much earlier than we generally do it, but might be okay here. People who had code:

plot_montage(montage, 10, True, ...)

could arguably benefit from having to updating their code to

plot_montage(montage, scale_factor=10, show_names=True, ...)

anyway because it's not at all obvious what the args are being used for in the first instance. (Whereas for something like the Epochs constructor you can reasonably do Epochs(raw, events, event_id) and it's pretty clear what's going on.)

So I think I'm okay with having the * right after montage here

[drammock]

So I think I'm okay with having the * right after montage here

agreed.

[cbrnr]

Done and ready for review.

[larsoner]

Looks good, thanks @cbrnr ! In principle we could add a test but I'm not sure it's worth it here.