[ENH] (Re)implement complex data support for Spectrum and SpectrumArray classes

[tsbinns]

Reference issue

Addresses #12728 & discussed in chat with @larsoner & @drammock

What does this implement/fix?

Idea is to:

• re-implement support for complex coefficients in (Epochs)Spectrum classes present before MAINT: Post-release deprecations #12265
• implement support for complex coefficients in (Epochs)SpectrumArray classes which was a WIP in [ENH, MRG] Add EpochsSpectrumArray and SpectrumArray classes  #11803 before decision to drop support was made
• also re-implements support for output="complex" as a kwarg in compute_psd() methods to bring API in line with compute_tfr() (@drammock this wasn't much more work beyond the other constructor changes)

As I understand, the decision to initially drop support was that complex coeff support does increase the complexity of the code somewhat and the use-cases perhaps weren't so clear. However, we would like to make some changes in MNE-Connectivity where having Spectrum objects support coefficients would be really useful (in addition to some uses mentioned in #11803).

Will include some comments for particular changes below. Also very open to discussion/suggestions.

Additional information

I can't take much credit for this since it's mainly re-implementing the work of others, so big thanks to the original authors @drammock @larsoner @alexrockhill!

[tsbinns]

Will add a changelog entry when the extent of the changes being made is clearer.

[tsbinns]

Couple failures for the to_data_frame method for the pip-pre tests I'll need to look into.

[larsoner]

Couple failures for the to_data_frame method for the pip-pre tests I'll need to look into.

If you want you can ignore them here. It looks unrelated to your changes. I can open a quick PR to fix them unless you're already on it!

[larsoner]

Oh wait they are related... I was looking into it anyway so pushed a tiny fix, but feel free to force-push over it if you have a better one

[drammock]

This looks pretty good! See my inline comments. Regarding this:

I can't take much credit for this since it's mainly re-implementing the work of others, so big thanks to the original authors @drammock @larsoner @alexrockhill!

I suggest adding those three as co-authors on at least one commit (adding them on 0d8d09c would have made the most sense, but in the end we squash-merge so it won't matter, any commit will do).

[drammock]

Do you think it's also worth adding a versionadded tag to the dimnames and weights params in the array classes?

Sure, we should.

[alexrockhill]

This looks great! One small comment.

Very xarray esque. Maybe long term might be worth adding as a dependency or making a simple class with data and dimension names with helpers but this doesn't seem too brittle without that so no need right now in my opinion.

[tsbinns]

Okay, have included version info for new params/attrs.

I also updated the Notes section of the array classes to reflect the changes in this PR:

mne-python/mne/utils/docs.py

Lines 2924 to 2934 in 7badfbf

	docdict["notes_spectrum_array"] = """
	If the data passed in is real-valued, it is assumed to represent spectral *power* (not
	amplitude, phase, etc), and downstream methods (such as
	:meth:`~mne.time_frequency.SpectrumArray.plot`) assume power data. If you pass in
	real-valued data that is not power, axis labels will be incorrect.
	If the data passed in is complex-valued, it is assumed to represent Fourier
	coefficients. Downstream plotting methods will treat the data as such, attempting to
	convert this to power before visualisation. If you pass in complex-valued data that is
	not Fourier coefficients, axis labels will be incorrect.
	"""

Before it explicitly suggested against pasing in coeffs.

It also suggested passing in e.g. amplitude or phase data could have unintended consequences beyond incorrect axis labels, however having gone through the code I don't see what these other effects could be. It's just the typical add/drop_channels, pick, to_data_frame that will work regardless.

Personally I don't think the docs need to be so cautious.

[tsbinns]

Sorry, realised some of my docs formatting was inconsistent with MNE conventions so have fixed now.

[drammock]

It also suggested passing in e.g. amplitude or phase data could have unintended consequences beyond incorrect axis labels, however having gone through the code I don't see what these other effects could be. It's just the typical add/drop_channels, pick, to_data_frame that will work regardless.

Personally I don't think the docs need to be so cautious.

I think the previous caution was reflecting "besides axis labels, we haven't thought carefully about what else might break if you pass ampl. instead of power". Since you've now read through the code and not found anything, I think your less stern warning is fine.

[drammock]

circleCI error is unrelated: looks like TownCrier can't handle Sphinx 7.4.7 ? Or maybe it's a transient thing 🤷

[drammock]

@tsbinns maybe now if you add a changelog entry, the towncrier failure will magically go away :)

[tsbinns]

@tsbinns maybe now if you add a changelog entry, the towncrier failure will magically go away :)

Unfortunately not...

[larsoner]

Looks like sphinx-contrib/sphinxcontrib-towncrier#92 I'll add a pin

[drammock]

@tsbinns is this ready to go in?

[tsbinns]

@drammock Just had a look back over and yes, seems like everything has been addressed!

[tsbinns]

Damn, that one test for Win 3.10 just ran over the time limit, but I don't have permission to re-run.

[larsoner]

Restarted!

[tsbinns]

Yay, thanks for all the help with this @drammock & @larsoner !

[larsoner]

Yay, thanks for all the help with this @drammock & @larsoner !

Now we can work on the mne-connectivity stuff :)