Fix backwards-incompatible changes to Brain.add_annotation

[wmvanvliet]

Pr #13931 dropped support for providing custom parcellations to Brain.add_annotation. Users may rely on this and so does MNE-RSA. We cannot do this without at least a deprecation cycle.

This PR puts back the awkward option of providing a tuple (labels, cmap). I understand this was probably dropped because no labels nor centroids are available when using this method, so hover behavior cannot work. Hence I added a deprecation warning to it. We can remove this option in due time.

Since it would be nice to be able to draw annotations not read from a file, but generated by some code, I added the ability to pass a list of Label objects. This does support centroids and labels and hence hover behavior.

No AI was used in the creation of this PR.

[larsoner]

Ahh sorry didn't mean to break something that was actively being used!

Agreed on deprecating, calling brain.add_label seems like a clearer path here. But if we want to support this going forward as well I'm okay with it, but we'd need to be clearer about how to construct and use the feature (it's still not totally clear to me, and the test doesn't do what the docstring says AFAICT).

For the actual deprecation step, a better change would probably be to very early do a short-circuit if not _path_like(annot): return self._old_add_annot(...) so that almost nothing has to change about the current code, the warning and to-go-away code lives in a separate helper. Then when we deprecate it's as easy as deleting that conditional plus the associated helper, rather than doing some surgery now.

[larsoner]

In the test it looks like it's specified just as a single label per hemisphere.

[wmvanvliet]

That was not the intention, it can just be a plain list. I renamed fname_label_rh -> fname_label2 to hopefully make it less confusing.

[larsoner]

Actually I guess it's the ctab part that was misleading... but in any case, yes feel free to push your changes and ping me and I'll look again!

[larsoner]

This will get emitted twice, which I guess is okay. But if we go with the single-path option it won't.

[larsoner]

Also, I'm happy to push the stuff above if it would help!

[wmvanvliet]

I'm sorry the test put you on the wrong foot. Labels have a .hemi field so there's no need to specify labels per hemisphere, it can just be a plain list of labels just like the docstring says. The test adds two labels just to have something of a list. brain.add_label only adds a single label, which is slow if you want to display an entire parcellation calling .add_label in a loop. We could also modify .add_label to support a list if that is clearer. Agreed on the new strategy for deprecation.