Skip to content

Conversation

@MaximeBICMTL
Copy link
Contributor

@MaximeBICMTL MaximeBICMTL commented Dec 5, 2025

Description

This PR does what the title says it does. The code is rather straightforward. The only somewhat tricky part is that the tests failed because the imported file was already present in Raisinbread. I added some overrides to remove existing physiological files, which is a little dirty IMO but should do the job for now.

@github-actions github-actions bot added the Language: Python Issue or PR related to the Python codebase label Dec 5, 2025
@MaximeBICMTL MaximeBICMTL force-pushed the bids_eeg_import_database_integration_tests branch 2 times, most recently from a0f5a82 to ed98eba Compare December 5, 2025 07:27
@MaximeBICMTL MaximeBICMTL added Area: CI Issue or PR related to continuous integration, including automated tests and static checks Complexity: Simple Issue or PR that should be simple to implement, review, or test Pipeline: BIDS importer PR or issue related to the BIDS importer labels Dec 5, 2025
@MaximeBICMTL MaximeBICMTL changed the title Add database checks to the EEG BIDS import integration tess Add database checks to the EEG BIDS import integration test Dec 5, 2025
@MaximeBICMTL MaximeBICMTL force-pushed the bids_eeg_import_database_integration_tests branch from f0529f1 to 08a97c2 Compare December 5, 2025 09:05
Comment on lines +59 to +60
'HardwareFilters': 'n/a',
'SoftwareFilters': 'n/a',
Copy link
Contributor Author

@MaximeBICMTL MaximeBICMTL Dec 5, 2025

Choose a reason for hiding this comment

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

Unrelated to this PR, but shouldn't we store NULL in the database instead of 'n/a' ?

Copy link
Contributor

Choose a reason for hiding this comment

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

The string 'n/a' is the BIDS-recognized version of NULL and these fields were pulled directly from the *_eeg.json spec

@MaximeBICMTL MaximeBICMTL force-pushed the bids_eeg_import_database_integration_tests branch from 08a97c2 to 27f034e Compare December 5, 2025 10:05
@MaximeBICMTL MaximeBICMTL force-pushed the bids_eeg_import_database_integration_tests branch 5 times, most recently from eaefb46 to d1803e9 Compare December 5, 2025 13:39
@MaximeBICMTL MaximeBICMTL added Complexity: Medium Issue or PR that requires a moderate effort or expertise to implement, review, or test and removed Complexity: Simple Issue or PR that should be simple to implement, review, or test labels Dec 5, 2025
@MaximeBICMTL MaximeBICMTL force-pushed the bids_eeg_import_database_integration_tests branch from d1803e9 to 1b26881 Compare December 5, 2025 14:08
@MaximeBICMTL MaximeBICMTL force-pushed the bids_eeg_import_database_integration_tests branch from 4645f70 to e04ba3a Compare December 14, 2025 13:09
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 approve these changes and new helper functions

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 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 Pipeline: BIDS importer PR or issue related to the BIDS importer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants