Skip to content

BIDS MRI import integration test (PR 3-bis)#1392

Open
MaximeBICMTL wants to merge 6 commits intoaces:bids_staging_branchfrom
MaximeBICMTL:bids_mri_import_integration_test
Open

BIDS MRI import integration test (PR 3-bis)#1392
MaximeBICMTL wants to merge 6 commits intoaces:bids_staging_branchfrom
MaximeBICMTL:bids_mri_import_integration_test

Conversation

@MaximeBICMTL
Copy link
Contributor

@MaximeBICMTL MaximeBICMTL commented Mar 16, 2026

Builds on top of #1389 (diff)

Description

The PR this builds on heavily refactors the BIDS MRI import code, which is obviously a risky move. This PR adds an integration test for the BIDS MRI importer.

Dataset

The bids dataset used is one I made myself using the DCC090_587630_V2 Raisinbread DICOMs, and using the BidsGuess suggestions provided by dcm2niix to arrange the files. This dataset need to be added to the testing imaging bucket for this PR to pass the new test.

The dataset looks as follow:

/data/loris/incoming/DCC090_587630_V2/
├── dataset_description.json
├── derivatives
│   ├── dataset_description.json
│   └── sub-DCC090
│       └── ses-V2
│           ├── eddy
│           │   ├── sub-DCC090_ses-V2_acq-epb5_dir-AP_run-7_dwi.bval
│           │   ├── sub-DCC090_ses-V2_acq-epb5_dir-AP_run-7_dwi.bvec
│           │   ├── sub-DCC090_ses-V2_acq-epb5_dir-AP_run-7_dwi.json
│           │   └── sub-DCC090_ses-V2_acq-epb5_dir-AP_run-7_dwi.nii.gz
│           ├── preprocessing
│           │   ├── sub-DCC090_ses-V2_dir-RL_run-9_dwi.json
│           │   └── sub-DCC090_ses-V2_dir-RL_run-9_dwi.nii.gz
│           └── topup
│               ├── sub-DCC090_ses-V2_acq-epb0_dir-AP_run-6_dwi.json
│               ├── sub-DCC090_ses-V2_acq-epb0_dir-AP_run-6_dwi.nii.gz
│               ├── sub-DCC090_ses-V2_acq-epb0_dir-AP_run-8_dwi.json
│               └── sub-DCC090_ses-V2_acq-epb0_dir-AP_run-8_dwi.nii.gz
├── participants.tsv
└── sub-DCC090
    └── ses-V2
        ├── anat
        │   ├── sub-DCC090_ses-V2_acq-spc3p2_run-4_T2w.json
        │   ├── sub-DCC090_ses-V2_acq-spc3p2_run-4_T2w.nii.gz
        │   ├── sub-DCC090_ses-V2_acq-tfl3p2_run-3_T1w.json
        │   ├── sub-DCC090_ses-V2_acq-tfl3p2_run-3_T1w.nii.gz
        │   ├── sub-DCC090_ses-V2_acq-tse2p2_run-2_echo-1_T2w.json
        │   ├── sub-DCC090_ses-V2_acq-tse2p2_run-2_echo-1_T2w.nii.gz
        │   ├── sub-DCC090_ses-V2_acq-tse2p2_run-2_echo-2_T2w.json
        │   └── sub-DCC090_ses-V2_acq-tse2p2_run-2_echo-2_T2w.nii.gz
        └── dwi
            ├── sub-DCC090_ses-V2_acq-epb0_dir-AP_run-5_dwi.bval
            ├── sub-DCC090_ses-V2_acq-epb0_dir-AP_run-5_dwi.bvec
            ├── sub-DCC090_ses-V2_acq-epb0_dir-AP_run-5_dwi.json
            └── sub-DCC090_ses-V2_acq-epb0_dir-AP_run-5_dwi.nii.gz

Comparison to main

This PR targets the reworked importer instead of the original. It is because the original importer currently does not pass the tests. It seems to suffer from path joins as string concatenations issues (sub-1234ses-456anat kind of issues...). And I honestly don't really want to go fix this kind of issues just to replace the whole code afterwards, especially since the import of bvals and bvecs also seems broken in the main code, as well as the import of MRI derivatives (but well, it does not work here either TBH, at least this PR provides a test dataset to implement it correctly in the future).

@github-actions github-actions bot added Language: Python Issue or PR related to the Python codebase Package: BIDS reader PR or issue related to the BIDS reader labels Mar 16, 2026
@MaximeBICMTL MaximeBICMTL added 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 Pipeline: BIDS importer PR or issue related to the BIDS importer labels Mar 16, 2026
@MaximeBICMTL MaximeBICMTL force-pushed the bids_mri_import_integration_test branch 2 times, most recently from d120e17 to 1b1d891 Compare March 19, 2026 07:41
@MaximeBICMTL MaximeBICMTL force-pushed the bids_mri_import_integration_test branch from 1b1d891 to 8696d87 Compare March 21, 2026 09:20
@MaximeBICMTL MaximeBICMTL changed the title BIDS MRI import integration test (PR 3) BIDS MRI import integration test (PR 3-bis) Mar 22, 2026
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 Package: BIDS reader PR or issue related to the BIDS reader Pipeline: BIDS importer PR or issue related to the BIDS importer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant