-
Notifications
You must be signed in to change notification settings - Fork 35
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
Fix dynamic <<>> runindex #213
Merged
marcelzwiers
merged 22 commits into
Donders-Institute:master
from
dzemanov:fix-dynamic-runindex
Jan 22, 2024
Merged
Changes from 12 commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
1da0e46
Incorporate BidsMapping
fd486f4
Make increment_runindex to not retroactively add run1, delete run fro…
6702aa9
Incorporate bids_mappings to dcm2niix2bids.py
9fbf3d8
Add rename_runless_to_run1
c6dde1d
Add add_bids_mappings
8dafc44
Incorporate bids_mappings and run1 handling to other plugins
40e1c83
Add drop_session_from_bids_mappings
9dd2d6b
Incorporate bids_mappings to bidscoiner
f847d1c
Fix windows file separator
c408fc9
Fix bids mappings for derivatives
feeed52
Add verbose log for writing bids mappings
d29115d
Use matched_runs instead of bids_mappings
2b92fd2
Move run['targets'] to run['datasource'].targets
marcelzwiers e3eb818
Simplify `updatemetadata()`
marcelzwiers 58caa9d
Minor tweak (use a set instead of a list as `datasource.targets`)
marcelzwiers c656975
Minor tweaks
marcelzwiers 6b6dd96
Bugfixes
marcelzwiers 71e5130
Add targets for the physio files (this was previously commited but go…
marcelzwiers 1115c6d
Bugfix
marcelzwiers 521d1b8
Refactor `bids.get_run_()` to inherit the datasource
marcelzwiers fe1e788
Fix provenance test for Windows file separators
marcelzwiers 2ce64e0
Minor tweak
marcelzwiers File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,7 +10,7 @@ | |
import json | ||
import shutil | ||
from bids_validator import BIDSValidator | ||
from typing import Union | ||
from typing import Union, List | ||
from pathlib import Path | ||
from bidscoin import bcoin, bids, lsdirs, due, Doi | ||
from bidscoin.utilities import physio | ||
|
@@ -228,6 +228,7 @@ def bidscoiner_plugin(session: Path, bidsmap: dict, bidsses: Path) -> Union[None | |
scans_table.index.name = 'filename' | ||
|
||
# Process all the source files or run subfolders | ||
matched_runs: List[dict] = [] | ||
sourcefile = Path() | ||
for source in sources: | ||
|
||
|
@@ -254,6 +255,7 @@ def bidscoiner_plugin(session: Path, bidsmap: dict, bidsses: Path) -> Union[None | |
continue | ||
|
||
LOGGER.info(f"--> Coining: {source}") | ||
matched_runs.append(run) | ||
|
||
# Create the BIDS session/datatype output folder | ||
suffix = datasource.dynamicvalue(run['bids']['suffix'], True, True) | ||
|
@@ -267,7 +269,7 @@ def bidscoiner_plugin(session: Path, bidsmap: dict, bidsses: Path) -> Union[None | |
bidsignore = bids.check_ignore(datasource.datatype, bidsmap['Options']['bidscoin']['bidsignore']) | ||
bidsname = bids.get_bidsname(subid, sesid, run, not bidsignore, runtime=True) | ||
bidsignore = bidsignore or bids.check_ignore(bidsname+'.json', bidsmap['Options']['bidscoin']['bidsignore'], 'file') | ||
bidsname = bids.increment_runindex(outfolder, bidsname, run, scans_table) | ||
bidsname = bids.increment_runindex(outfolder, bidsname, run) | ||
jsonfiles = set() # Set -> Collect the associated json-files (for updating them later) -- possibly > 1 | ||
|
||
# Check if the bidsname is valid | ||
|
@@ -312,6 +314,7 @@ def bidscoiner_plugin(session: Path, bidsmap: dict, bidsses: Path) -> Union[None | |
if not list(outfolder.glob(f"{bidsname}.*nii*")): continue | ||
|
||
jsonfiles.update(outfolder.glob(f"{bidsname}.json")) # add existing created json files: bidsname.json | ||
run["targets"].update(outfolder.glob(f"{bidsname}.*[!json]")) # add files created using this bidsmap run-item (except sidecars) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line is all that is need when targets are added to the datasource |
||
|
||
# Handle the ABCD GE pepolar sequence | ||
extrafile = list(outfolder.glob(f"{bidsname}a.nii*")) | ||
|
@@ -419,12 +422,13 @@ def bidscoiner_plugin(session: Path, bidsmap: dict, bidsses: Path) -> Union[None | |
LOGGER.warning(f"The {newbidsname} image is a derivate / not BIDS-compliant -- you can probably delete it safely and update {scans_tsv}") | ||
|
||
# Save the NIfTI file with the newly constructed name | ||
newbidsname = bids.increment_runindex(outfolder, newbidsname, run, scans_table) # Update the runindex now that the acq-label has changed | ||
newbidsname = bids.increment_runindex(outfolder, newbidsname, run) # Update the runindex now that the acq-label has changed | ||
newbidsfile = outfolder/newbidsname | ||
LOGGER.verbose(f"Found dcm2niix {postfixes} postfixes, renaming\n{dcm2niixfile} ->\n{newbidsfile}") | ||
if newbidsfile.is_file(): | ||
LOGGER.warning(f"Overwriting existing {newbidsfile} file -- check your results carefully!") | ||
dcm2niixfile.replace(newbidsfile) | ||
run["targets"].add(newbidsfile) | ||
|
||
# Rename all associated files (i.e. the json-, bval- and bvec-files) | ||
oldjsonfile = dcm2niixfile.with_suffix('').with_suffix('.json') | ||
|
@@ -458,6 +462,8 @@ def bidscoiner_plugin(session: Path, bidsmap: dict, bidsses: Path) -> Union[None | |
LOGGER.verbose(f"Removing BIDS-invalid b0-file: {bfile} -> {jsonfile}") | ||
metadata[ext[1:]] = bdata.values.tolist() | ||
bfile.unlink() | ||
if bfile in run["targets"]: | ||
run["targets"].remove(bfile) | ||
|
||
# Save the meta-data to the json sidecar-file | ||
with jsonfile.open('w') as json_fid: | ||
|
@@ -486,6 +492,9 @@ def bidscoiner_plugin(session: Path, bidsmap: dict, bidsses: Path) -> Union[None | |
scanpath = outputfile[0].relative_to(bidsses) | ||
scans_table.loc[scanpath.as_posix(), 'acq_time'] = acq_time | ||
|
||
# Handle dynamic index for run-1 | ||
bids.rename_runless_to_run1(matched_runs, scans_table) | ||
|
||
# Write the scans_table to disk | ||
LOGGER.verbose(f"Writing acquisition time data to: {scans_tsv}") | ||
scans_table.sort_values(by=['acq_time','filename'], inplace=True) | ||
|
This file contains 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
This file contains 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks a bit like a hack, I suspect it is not needed if we add
targets
to the datasource instead of to the run. The datasource as part of the run is already hacky, but at least then it is all in one place. Moreover, cleaning up the datasource from the run is already implemented (e.g. when saving the bidsmap). I will make some commits to your PR so you can see what I mean