Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MRG] Add support for eeglab export #1187

Merged
merged 8 commits into from Nov 16, 2023
Merged

Conversation

laemtl
Copy link
Contributor

@laemtl laemtl commented Nov 3, 2023

PR Description

Add support for EEGLAB files (for preloaded .set files) in write_raw_bids
HDF5 support is not provided yet.

Closes #1152

Merge checklist

Maintainer, please confirm the following before merging.
If applicable:

  • All comments are resolved
  • This is not your own PR
  • All CIs are happy
  • PR title starts with [MRG]
  • whats_new.rst is updated
  • New contributors have been added to CITATION.cff
  • PR description includes phrase "closes <#issue-number>"

@sappelhoff
Copy link
Member

Thanks!

Please add a test to this existing testing infrastructure:

@pytest.mark.parametrize("dir_name, fname, reader", test_eegieeg_data)
@pytest.mark.filterwarnings(
warning_str["nasion_not_found"],
warning_str["brainvision_unit"],
warning_str["channel_unit_changed"],
warning_str["cnt_warning1"],
warning_str["cnt_warning2"],
warning_str["no_hand"],
warning_str["no_montage"],
)
@testing.requires_testing_data
def test_eegieeg(dir_name, fname, reader, _bids_validate, tmp_path):

you should be able to simply add to these data:

# parametrization for testing converting file formats for EEG/iEEG
test_converteeg_data = [
(
"Persyst",
"BrainVision",
"sub-pt1_ses-02_task-monitor_acq-ecog_run-01_clip2.lay",
_read_raw_persyst,
), # noqa
("NihonKohden", "BrainVision", "MB0400FU.EEG", _read_raw_nihon),
("CNT", "BrainVision", "scan41_short.cnt", _read_raw_cnt),
(
"curry",
"BrainVision",
"test_bdf_stim_channel Curry 8.cdt",
_read_raw_curry,
), # noqa
(
"Persyst",
"EDF",
"sub-pt1_ses-02_task-monitor_acq-ecog_run-01_clip2.lay",
_read_raw_persyst,
), # noqa
("NihonKohden", "EDF", "MB0400FU.EEG", _read_raw_nihon),
("CNT", "EDF", "scan41_short.cnt", _read_raw_cnt),
("curry", "EDF", "test_bdf_stim_channel Curry 8.cdt", _read_raw_curry),
]

Please also add an entry to the changelog: https://github.com/mne-tools/mne-bids/blob/main/doc/whats_new.rst

Copy link

codecov bot commented Nov 3, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (5ad1db4) 97.64% compared to head (67dc2a5) 97.64%.

Files Patch % Lines
mne_bids/write.py 90.90% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1187      +/-   ##
==========================================
- Coverage   97.64%   97.64%   -0.01%     
==========================================
  Files          40       40              
  Lines        8675     8689      +14     
==========================================
+ Hits         8471     8484      +13     
- Misses        204      205       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sappelhoff
Copy link
Member

HDF5 support is not provided yet.

perhaps you could open an issue on https://github.com/jackz314/eeglabio and ask whether support could be added there.

I personally think it'd be great to include that.

@laemtl
Copy link
Contributor Author

laemtl commented Nov 7, 2023

HDF5 support is not provided yet.

perhaps you could open an issue on https://github.com/jackz314/eeglabio and ask whether support could be added there.

I personally think it'd be great to include that.

I totally agree :)

@sappelhoff
Copy link
Member

@laemtl gentle reminder :-)

@sappelhoff sappelhoff self-assigned this Nov 16, 2023
@sappelhoff
Copy link
Member

perhaps you could open an issue on https://github.com/jackz314/eeglabio and ask whether support could be added there.

I personally think it'd be great to include that.

I've done that now, please see: jackz314/eeglabio#10

@sappelhoff
Copy link
Member

I have also pushed some commits to point you in the right direction, please feel free to continue once you have time.

@sappelhoff sappelhoff removed their assignment Nov 16, 2023
@sappelhoff
Copy link
Member

PS: If you do want to continue work on this, you will need to sync your local git repo with your branch again, because I force pushed some changes.

You can do it (locally) like this:

  • ensure there are no pending changes that you'd like to keep
  • git reset --hard origin/eeglab.export

@sappelhoff sappelhoff merged commit 7708a3f into mne-tools:main Nov 16, 2023
16 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Export to EEGLAB format?
2 participants