Store n_cycles and time_bandwidth params in *TFR objects

[tsbinns]

Describe the new feature or enhancement

Discussed partly with @larsoner.

MNE-Connectivity is being updated to allow connectivity computations using complex coefficients stored in EpochsTFR objects (mne-tools/mne-connectivity#232). For the multitaper method, TFR coefficients have a tapers dimension that need to be aggregated over when computing the CSD. These weights are not stored in the *TFR objects, and computing these weights requires knowledge of the time_bandwidth and n_cycles used to estimate the TFR. However, currently these parameters are not stored in *TFR objects.

If we want to accurately compute these weights in MNE-Connectivity, we would currently require the user to pass these parameters in addition to the EpochsTFR object. It would be more convenient if this information could also be read from the EpochsTFR object.

Describe your proposed implementation

There were two ideas:

• Either store this information directly as attributes, e.g., tfr.n_cycles and tfr.time_bandwidth
• Or store these params as a dict to minimise clutter in the *TFR namespace, e.g., tfr.estimation_parameters = {'mt_bandwidth': 4, 'n_cycles': 5}

Describe possible alternatives

An alternative would be to just store the taper weights in the *TFR objects, which is similar to how this is now handled for *Spectrum objects. However, the weights are not returned by tfr_array_multitaper, so they would still need to be recomputed.

It's perhaps simpler if we just handle this in MNE-Connectivity using the stored estimation params.

What do people think is the best choice?

[drammock]

An alternative would be to just store the taper weights in the *TFR objects, which is similar to how this is now handled for *Spectrum objects. However, the weights are not returned by tfr_array_multitaper, so they would still need to be recomputed.

Since we already store weights for spectrum objects, I'm inclined to store weights (not estimation params) for TFRs too. That saves having to recompute them when users pass MNE objects (not arrays) to the connectivity functions, and is more internally consistent. (this assumes that we don't anticipate needing n_cycles or mt_bandwidth for other purposes... I suppose to be maximally future-proof we could store the weights and the estimation params)

When users pass NumPy arrays to the conn functions, they can/should expect that they'll need to provide some extra information, so IMO it's fine to ask them to pass in estimation params (n_cycles and mt_bandwidth) and we calc the weights for them at that time... even though it's a bit error-prone to trust that the user passes the same values to both the TFR function and the connectivity function.

Another option would be adding return_weights=False to tfr_array_multitaper, so that folks can use the array-based connectivity API as long as they're willing to re-compute the TFR with return_weights=True first. That feels slightly worse to me than just asking them to pass in the estimation params.

[tsbinns]

Since we already store weights for spectrum objects, I'm inclined to store weights (not estimation params) for TFRs too. That saves having to recompute them when users pass MNE objects (not arrays) to the connectivity functions, and is more internally consistent.

Sure, I see your reasoning, especially to keep it consistent with *Spectrum objects. I'll open a PR for this.

When users pass NumPy arrays to the conn functions, they can/should expect that they'll need to provide some extra information, so IMO it's fine to ask them to pass in estimation params (n_cycles and mt_bandwidth) and we calc the weights for them at that time... even though it's a bit error-prone to trust that the user passes the same values to both the TFR function and the connectivity function.

Just to clarify, for the proposed connectivity function changes, users would still only be able to pass in arrays of epoched timeseries data, not epoched coefficients. If we change to store weights in *TFR objects, then n_cycles and mt_bandwidth params could be completely ignored for *TFR data, but for Epochs and arrays of epoched timeseries data they would still function in the same way (to determine the TFR estimation).

[drammock]

Just to clarify, for the proposed connectivity function changes, users would still only be able to pass in arrays of epoched timeseries data, not epoched coefficients. If we change to store weights in *TFR objects, then n_cycles and mt_bandwidth params could be completely ignored for *TFR data, but for Epochs and arrays of epoched timeseries data they would still function in the same way (to determine the TFR estimation).

Ah OK, so it's for any non-spectral/spectrotemporal data (whether in Epochs object form or array form) where they need to pass the estimation params. That makes sense.

[tsbinns]

Ah OK, so it's for any non-spectral/spectrotemporal data (whether in Epochs object form or array form) where they need to pass the estimation params. That makes sense.

Yeah, exactly.