-
Notifications
You must be signed in to change notification settings - Fork 52
Incremental BIDS import #1211
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
Closed
maximemulder
wants to merge
6
commits into
aces:main
from
maximemulder:2024-12-02_incremental_import_bids
Closed
Incremental BIDS import #1211
maximemulder
wants to merge
6
commits into
aces:main
from
maximemulder:2024-12-02_incremental_import_bids
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
8ccc017 to
751bf02
Compare
d48df6c to
2ce3fb9
Compare
8033f6e to
b3aeb95
Compare
00d3c65 to
d2caaab
Compare
30a3c2c to
f27561a
Compare
6a4b0bf to
c7f6407
Compare
83afa11 to
56a4c42
Compare
56a4c42 to
84dcebc
Compare
1d3ee78 to
3494167
Compare
a5a5a1a to
92bc1fd
Compare
Contributor
|
Hello, a few comments:
|
bids session dataclass bids participants dataclass factorize combination iteration fix layout ignore skip files already inserted wip fix mri path join rebase commit
8f2f244 to
b533b6b
Compare
b533b6b to
646dd27
Compare
This was referenced Jul 1, 2025
Merged
|
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 20 days. |
Contributor
|
Replaced by #1325 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
BIDS dataset import script rewrite
List of new features
--create_candidateor--create_sessionoptions are now transactional. Either all candidates and sessions are created without error, or none is created if an error occurs (due to incorrect input data).--create_candidateoption now looks for sites and projects using both names and aliases, in that order.List of breaking changes
bids_import.pytoimport_bids_dataset.py. This should better follow the LORIS-MRI scripts naming conventions.dataset_description.jsonfile is absent. This file is a required BIDS file, and was already needed unless the--nocopyoption was used. It is also trivial to write anyway.--type=derivativesor--no_copyoptions are used. It seems to me that the current code was not tested and probably not working, so it is better to have a hard error rather than an untested and potentially wrong implementation IMO.--idsvalidationoption has been removed. This option allowed to validate the BIDS directory name according to apscid_candid_blablablaformat. This is obviously esoteric and overly-specific as BIDS dataset can contain several participants, and their identifiers should be retrieved from the subject directory names or theparticipants.tsvfile instead.--create_candidateoption no longer tries to find the site by matching site aliases against participants IDs. This approach can give false positives if a similar (and not even necessarily equal) alias as a project. Thesiteparticipants.tsvcolumn is now mandatory to create candidates.List of major architectural changes
Note that these changes only apply to the generic BIDS import code (subjects and sessions), and the MRI import code. It does not apply to EEG import code.
print.lib.import_bids_datasetmodule.lib.imaging_libmodule to replace the oldlib.imagingmodule. Unlikelib.imaging, the new module is fully typed, uses the new ORM database abstraction, and is divided into submodules related to different kinds of imaging data (BIDS, DICOM, file parameters...).Future works
Other comments
This PR is very large so it will likely be divided in smaller PRs to be reviewed and merged.