mne-tools / mne-connectivity

Connectivity algorithms that leverage the MNE-Python API.
https://mne.tools/mne-connectivity/dev/index.html
BSD 3-Clause "New" or "Revised" License
66 stars 34 forks source link

fix for loading with missing event_id #110

Closed drammock closed 1 year ago

drammock commented 1 year ago

PR Description

closes #109. There should probably be a test too, but I didn't even see a tests/test_io.py file and at the moment I haven't time to do a proper job of that.

Merge checklist

Maintainer, please confirm the following before merging:

drammock commented 1 year ago

This CI error: https://github.com/mne-tools/mne-connectivity/actions/runs/3423420815/jobs/5701993749#step:10:54 should have been fixed with matplotlib 3.6.2 (released 7 days ago). It's caused by a change in PySide 6.4, which is quite recent (maybe 1 month old?). I'll push the same fix from https://github.com/mne-tools/mne-python/pull/11246

drammock commented 1 year ago

weird, the updated spec in requirements.txt isn't getting followed? @larsoner any idea what's wrong here?

https://github.com/mne-tools/mne-connectivity/actions/runs/3432799079/jobs/5722434556#step:5:41

larsoner commented 1 year ago

For the 3.7 build I'm guessing there is no fixed wheel, maybe it's 3.8 only? In any case it installs 3.5.something.

I would just bump this build to 3.8. The failures in other builds look legitimate and related like

_________ test_time_resolved_spectral_conn_regression[multitaper-plv] __________
mne_connectivity/spectral/tests/test_spectral.py:590: in test_time_resolved_spectral_conn_regression
    assert_array_almost_equal(conn_data, test_conn)
E   AssertionError: 
E   Arrays are not almost equal to 6 decimals
E   
E   Mismatched elements: 35988 / 36000 (100%)
E   Max absolute difference: 0.02373527
E   Max relative difference: 0.08923911
E    x: array([[[[0.390603, 0.393248, 0.395886, ..., 0.179899, 0.[176](https://github.com/mne-tools/mne-connectivity/actions/runs/3432799079/jobs/5722434609#step:11:177)772,
E             0.173664],
E            [0.24873 , 0.250642, 0.252551, ..., 0.091652, 0.089639,...
E    y: array([[[[0.396488, 0.399161, 0.401826, ..., 0.[179](https://github.com/mne-tools/mne-connectivity/actions/runs/3432799079/jobs/5722434609#step:11:180)551, 0.176434,
E             0.173338],
E            [0.257082, 0.259085, 0.261087, ..., 0.090343, 0.088305,...
drammock commented 1 year ago

For the 3.7 build I'm guessing there is no fixed wheel, maybe it's 3.8 only? In any case it installs 3.5.something.

To me the problem is not the matplotlib version, it's that I specify PySide6!=6.3.0,!=6.4.0 and it installs 6.4.0 anyway.

The failures in other builds look legitimate and related

Legitimate, yes. Related to this PR's changes? I'm not so sure.

larsoner commented 1 year ago

You probably need 6.4.0.1 in the line, too

https://github.com/mne-tools/mne-python/blob/4ef3a88c7c1ea29b4925cece2072ad5c5d826b82/requirements.txt#L12

adam2392 commented 1 year ago

Thanks @drammock !