Skip to content

Conversation

@MaximeBICMTL
Copy link
Contributor

@MaximeBICMTL MaximeBICMTL commented Nov 22, 2025

Extracted from #1325

Description

Add integration tests for EEG BIDS import.

For the tests to pass, this PR requires to add the Face13 BIDS dataset to the test S3 bucket as incoming/Face13. Only the sub-OTT166 subject is required, the participants.tsv file should contain this:

participant_id	cohort	site	project
sub-OTT166	Fresh	OTT	PUMP

This PR also adds the ability to use project aliases when using --createsession in the current BIDS importer to be consistent with the new BIDS importer.

Description 2

Since the tests did not work at first, I also added a bugfix in this PR to make the code work with the most recent mne version (1.11.0). Note that the code no longer works with the previous version (1.10.x or less).

@github-actions github-actions bot added the Language: Python Issue or PR related to the Python codebase label Nov 22, 2025
@MaximeBICMTL MaximeBICMTL added Area: CI Issue or PR related to continuous integration, including automated tests and static checks Pipeline: BIDS importer PR or issue related to the BIDS importer Package: EEG chunker PR or issue related to the EEG chunker Complexity: Medium Issue or PR that requires a moderate effort or expertise to implement, review, or test Language: Python Issue or PR related to the Python codebase and removed Language: Python Issue or PR related to the Python codebase labels Nov 22, 2025
@MaximeBICMTL MaximeBICMTL force-pushed the add_bids_import_eeg_integration_test branch from 52ffb0b to d1d118e Compare November 22, 2025 06:00
@MaximeBICMTL MaximeBICMTL force-pushed the add_bids_import_eeg_integration_test branch from d074bfc to 37e28d6 Compare November 25, 2025 02:20
@MaximeBICMTL
Copy link
Contributor Author

MaximeBICMTL commented Nov 25, 2025

@jeffersoncasimir I added two commits to this PR:

  • 935e4df reworks the check_file_tree test function into assert_files_exist as the former had terrible debugging ergonomics when it was failing.
  • 37e28d6 updates the arguments of mne_edf._get_info() to account for the latest mne update two days ago that changed the signature of this function (which is why your second run failed).

@MaximeBICMTL MaximeBICMTL added the Category: Bug Issue or PR that aims to report or fix a bug label Nov 25, 2025
Copy link
Contributor

@jeffersoncasimir jeffersoncasimir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am getting this error:

Traceback (most recent call last):
  File "/opt/Loris-MRI/bin/mri/python/loris_eeg_chunker/edf_to_chunks.py", line 43, in <module>
    file_type=mne_edf.FileType.EDF,
              ^^^^^^^^^^^^^^^^
AttributeError: module 'mne.io.edf.edf' has no attribute 'FileType'

mne==1.10.2

Works with latest version (1.11.0). Changed to approved

@jeffersoncasimir jeffersoncasimir self-requested a review November 26, 2025 18:26
@cmadjar cmadjar merged commit 1f6996f into aces:main Nov 26, 2025
10 checks passed
@MaximeBICMTL MaximeBICMTL added this to the 28.0.0 milestone Dec 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: CI Issue or PR related to continuous integration, including automated tests and static checks Category: Bug Issue or PR that aims to report or fix a bug Complexity: Medium Issue or PR that requires a moderate effort or expertise to implement, review, or test Language: Python Issue or PR related to the Python codebase Package: EEG chunker PR or issue related to the EEG chunker Pipeline: BIDS importer PR or issue related to the BIDS importer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants