-
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
Draft
maximemulder
wants to merge
6
commits into
aces:main
Choose a base branch
from
maximemulder:2024-12-02_incremental_import_bids
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Incremental BIDS import #1211
maximemulder
wants to merge
6
commits into
aces:main
from
maximemulder:2024-12-02_incremental_import_bids
+2,430
−2,079
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
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
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_candidate
or--create_session
options 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_candidate
option now looks for sites and projects using both names and aliases, in that order.List of breaking changes
bids_import.py
toimport_bids_dataset.py
. This should better follow the LORIS-MRI scripts naming conventions.dataset_description.json
file is absent. This file is a required BIDS file, and was already needed unless the--nocopy
option was used. It is also trivial to write anyway.--type=derivatives
or--no_copy
options 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.--idsvalidation
option has been removed. This option allowed to validate the BIDS directory name according to apscid_candid_blablabla
format. 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.tsv
file instead.--create_candidate
option 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. Thesite
participants.tsv
column 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_dataset
module.lib.imaging_lib
module to replace the oldlib.imaging
module. 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.