MRG, ENH: Add arbitrary connectivity for stats #7916
Conversation
drammock
left a comment
drammock
left a comment
There was a problem hiding this comment.
code looks reasonable. Just the two comments.
tutorials/stats-sensor-space/plot_stats_cluster_1samp_test_time_frequency.py
Show resolved
Hide resolved
|
@larsoner Great, I'll take a look within 24h! |
| if p_val <= 0.05: | ||
| T_obs_plot[c] = T_obs[c] | ||
|
|
||
| # Just plot one channel's data |
There was a problem hiding this comment.
I'd write that we find indices of the maximum absolute t value
| epochs_power = tfr_epochs.data | ||
|
|
||
| ############################################################################### | ||
| # Define connectivity for statistics |
There was a problem hiding this comment.
I must say that the naming for adjacency is a bit unfortunate in mne. I was a few times in a situation when I had to search online for mne function that finds adjacency. Because it has connectivity in its name finding it was a bit difficult: I was getting lots of results related to actual connectivity (coherence, phase slope index etc.) but not the function I needed.
|
We could rename to adjacency in another PR. I agree it's bad naming to have this collide with functional and effective connectivity, I almost named this combine_adjacency but the deprecation bothered me. @agramfort okay to rename via deprecation spatio_src_connectivity and related functions to spatial_src_adjacency, and clustering arguments to adjacency instead of connectivity? |
|
how is it called in other packages?
|
|
Based on a minimal-effort searching (others feel free to chime in if you know of others or better for these packages)
To me "adjacency" is the winner based on this |
|
ok then
… |
|
Okay, deprecations complete for |
mne/stats/tests/test_adjacency.py
Outdated
| # for now | ||
| assert np.in1d(conn, [0, 1, 2, 3]).all() | ||
| assert conn.shape == conn_sk.shape | ||
| assert_array_equal(conn, conn) |
There was a problem hiding this comment.
both arrays are the same object here
|
Looks good! And I'm glad it's |
|
Good catch @mmagnuski, that errant naming was missing an issue with the diagonals being repeated such that they were no longer unity values (they were equal to |
* upstream/master: MRG: Prepare migration to PyVista 0.25 (mne-tools#7791) MAINT: Simpler VTK [circle front] (mne-tools#7931) MRG, ENH: Add arbitrary connectivity for stats (mne-tools#7916)
4 participants
Arguably part of #4859, originally brought up in #5745, resurfaced on the mailing list today.
Closes #7915
@mmagnuski you might have asked about this sort of thing before, interested in reviewing and/or trying?
In theory at some point we could probably exploit a lattice structure to avoid a lot of unnecessary checks (just like we do in the
*_temporalvariants of our stats functions), but this is at least a good start that should allow people to do these sorts of analyses even if they are slower than they could be.