Skip to content

Commit

Permalink
Refactoring (-> github PR #195):
Browse files Browse the repository at this point in the history 
- Move core dynamic run value code from the plugins to the bids module
- Move `add_run1_keyval()` code to `increment_runindex` (as they should always be executed in conjunction)
Bugfix dcm2niix (add_run1_keyval: bidsname -> newbidsname)
  • Loading branch information
@marcelzwiers
marcelzwiers committed Aug 18, 2023
1 parent baad2f8 commit 3c9b896
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 103 deletions.
67 changes: 34 additions & 33 deletions bidscoin/bids.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import tempfile
import warnings
import fnmatch
import pandas as pd
from functools import lru_cache
from pathlib import Path
from typing import Union, List, Tuple
Expand Down Expand Up @@ -1870,53 +1871,53 @@ def insert_bidskeyval(bidsfile: Union[str, Path], bidskey: str, newvalue: str, v
return newbidsfile


def add_run1_keyval(outfolder: Union[Path, str], bidsname: str, scans_table: DataFrame, bidsses: Path) -> None:
def increment_runindex(outfolder: Path, bidsname: str, run: dict, scans_table: DataFrame=pd.DataFrame()) -> Union[Path, str]:
"""
Adds run-1 key to files with bidsname that don't have run index. Updates scans respectively.
Checks if a file with the same bidsname already exists in the folder and then increments the dynamic runindex
(if any) until no such file is found. If file already exists but has no run index, starts with run-2 and adds
run-1 key to run-1 files with bidsname that don't have run index. Updates scans table respectively
:param outfolder: The path where files with bidsname without run index are searched for
:param bidsname: The bidsname of files to search for, has runindex
:param scans_table: Scans dataframe
:param bidsses: The full-path name of the BIDS output `sub-/ses-` folder
:return: Nothing
:param outfolder: The full pathname of the bids output folder
:param bidsname: The bidsname with a provisional runindex
:param run: The run mapping with the BIDS key-value pairs
:param scans_table: BIDS scans.tsv dataframe with all filenames and acquisition timestamps
:return: The bidsname with the original or incremented runindex
"""
old_bidsname = insert_bidskeyval(bidsname, 'run', '', False)
new_bidsname = insert_bidskeyval(bidsname, 'run', '1', False)
scanpath = outfolder.relative_to(bidsses)
for file in outfolder.glob(old_bidsname + '.*'):
ext = ''.join(file.suffixes)
file.rename((outfolder / new_bidsname).with_suffix(ext))
# change row name in scans
if ext in ('.nii.gz', '.nii'):
scans_table.rename(
index={(scanpath / old_bidsname).with_suffix(ext).as_posix(): (scanpath / new_bidsname).with_suffix(ext).as_posix()},
inplace=True)

# Check input
runval = run['bids'].get('run')
runval = str(runval) if runval else ''
if not runval.startswith('<<') and not runval.endswith('>>'):
return bidsname

def increment_runindex(bidsfolder: Path, bidsname: str, ext: str='.*') -> Union[Path, str]:
"""
Checks if a file with the same bidsname already exists in the folder and then increments the runindex (if any)
until no such file is found. If file already exists but has no run index, starts with run-2.
:param bidsfolder: The full pathname of the bidsfolder
:param bidsname: The bidsname with a provisional runindex
:param ext: The file extension for which the runindex is incremented (default = '.*')
:return: The bidsname with the incremented runindex
"""
if 'run' not in bidsname: # (default bidsname for dynamic value <<>> is without run)
# Catch run-less bidsnames from <<>> dynamic run-values
if 'run' not in bidsname:
run1_bidsname = insert_bidskeyval(bidsname, 'run', '1', False)
if list(bidsfolder.glob(run1_bidsname + ext)):
bidsname = run1_bidsname # run1 exists

while list(bidsfolder.glob(bidsname + ext)):
if list(outfolder.glob(f"{run1_bidsname.split('.')[0]}.*")):
bidsname = run1_bidsname # There is more than 1 run, i.e. run-1 exists and should be incremented

# Increment the run-index if the bidsfile already exists
while list(outfolder.glob(f"{bidsname.split('.')[0]}.*")):
runindex = get_bidsvalue(bidsname, 'run')
if not runindex:
# bidsname (run-1) doesn't have run index yet, start with run-2 for this one
bidsname = insert_bidskeyval(bidsname, 'run', '2', False)
else:
bidsname = get_bidsvalue(bidsname, 'run', str(int(runindex) + 1))

# Adds run-1 key to files with bidsname that don't have run index. Updates scans respectively
if runval == '<<>>' and 'run-2' in bidsname:
old_bidsname = insert_bidskeyval(bidsname, 'run', '', False)
new_bidsname = insert_bidskeyval(bidsname, 'run', '1', False)
for file in outfolder.glob(f"{old_bidsname}.*"):
ext = ''.join(file.suffixes)
file.rename((outfolder/new_bidsname).with_suffix(ext))

# Change row name in the scans table
if ext in ('.nii.gz', '.nii') and f"{outfolder.name}/{old_bidsname}{ext}" in scans_table.index:
scans_table.rename(index={f"{outfolder.name}/{old_bidsname}{ext}":
f"{outfolder.name}/{new_bidsname}{ext}"}, inplace=True) # NB: '/' as_posix

return bidsname


Expand Down
12 changes: 2 additions & 10 deletions bidscoin/plugins/dcm2niix2bids.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,12 +254,7 @@ def bidscoiner_plugin(session: Path, bidsmap: dict, bidsses: Path) -> 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')
runindex = run['bids'].get('run')
runindex = str(runindex) if runindex else ''
if runindex.startswith('<<') and runindex.endswith('>>'):
bidsname = bids.increment_runindex(outfolder, bidsname)
if runindex == '<<>>' and 'run-2' in bidsname:
bids.add_run1_keyval(outfolder, bidsname, scans_table, bidsses)
bidsname = bids.increment_runindex(outfolder, bidsname, run, scans_table)
jsonfiles = [(outfolder/bidsname).with_suffix('.json')] # List -> Collect the associated json-files (for updating them later) -- possibly > 1

# Check if the bidsname is valid
Expand Down Expand Up @@ -409,10 +404,7 @@ def bidscoiner_plugin(session: Path, bidsmap: dict, bidsses: Path) -> 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
if runindex.startswith('<<') and runindex.endswith('>>'):
newbidsname = bids.increment_runindex(outfolder, newbidsname, '') # Update the runindex now that the acq-label has changed
if runindex == '<<>>' and 'run-2' in bidsname:
bids.add_run1_keyval(outfolder, bidsname, scans_table, bidsses)
newbidsname = bids.increment_runindex(outfolder, newbidsname, run, scans_table) # 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():
Expand Down
9 changes: 2 additions & 7 deletions bidscoin/plugins/nibabel2bids.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,13 +212,8 @@ def bidscoiner_plugin(session: Path, bidsmap: dict, bidsses: Path) -> 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')
runindex = run['bids'].get('run')
runindex = str(runindex) if runindex else ''
if runindex.startswith('<<') and runindex.endswith('>>'):
bidsname = bids.increment_runindex(outfolder, bidsname)
if runindex == '<<>>' and 'run-2' in bidsname:
bids.add_run1_keyval(outfolder, bidsname, scans_table, bidsses)
bidsfile = (outfolder/bidsname).with_suffix(ext)
bidsname = bids.increment_runindex(outfolder, bidsname, run, scans_table)
bidsfile = (outfolder/bidsname).with_suffix(ext)

# Check if the bidsname is valid
bidstest = (Path('/')/subid/sesid/datasource.datatype/bidsname).with_suffix('.json').as_posix()
Expand Down
25 changes: 10 additions & 15 deletions bidscoin/plugins/pet2bids.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ def bidscoiner_plugin(session: Path, bidsmap: dict, bidsses: Path) -> None:
LOGGER.exception(f"Unsupported dataformat '{dataformat}'")

# Read or create a scans_table and tsv-file
scans_tsv = bidsses / f"{subid}{'_' + sesid if sesid else ''}_scans.tsv"
scans_tsv = bidsses/f"{subid}{'_' + sesid if sesid else ''}_scans.tsv"
scans_table = pd.DataFrame()
if scans_tsv.is_file():
scans_table = pd.read_csv(scans_tsv, sep='\t', index_col='filename')
Expand Down Expand Up @@ -246,37 +246,32 @@ def bidscoiner_plugin(session: Path, bidsmap: dict, bidsses: Path) -> None:
# Create the BIDS session/datatype output folder
suffix = datasource.dynamicvalue(run['bids']['suffix'], True, True)
if suffix in bids.get_derivatives(datasource.datatype):
outfolder = bidsfolder / 'derivatives' / manufacturer.replace(' ', '') / subid / sesid / datasource.datatype
outfolder = bidsfolder/'derivatives'/manufacturer.replace(' ', '')/subid/sesid/datasource.datatype
else:
outfolder = bidsses / datasource.datatype
outfolder = bidsses/datasource.datatype
outfolder.mkdir(parents=True, exist_ok=True)

# Compose the BIDS filename using the matched run
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')
runindex = run['bids'].get('run')
runindex = str(runindex) if runindex else ''
if runindex.startswith('<<') and runindex.endswith('>>'):
bidsname = bids.increment_runindex(outfolder, bidsname)
if runindex == '<<>>' and 'run-2' in bidsname:
bids.add_run1_keyval(outfolder, bidsname, scans_table, bidsses)
bidsname = bids.increment_runindex(outfolder, bidsname, run, scans_table)

# Check if the bidsname is valid
bidstest = (Path('/') / subid / sesid / datasource.datatype / bidsname).with_suffix('.json').as_posix()
bidstest = (Path('/')/subid/sesid/datasource.datatype/bidsname).with_suffix('.json').as_posix()
isbids = BIDSValidator().is_bids(bidstest)
if not isbids and not bidsignore:
LOGGER.warning(f"The '{bidstest}' output name did not pass the bids-validator test")

# Check if file already exists (-> e.g. when a static runindex is used)
if (outfolder / bidsname).with_suffix('.json').is_file():
LOGGER.warning(f"{outfolder / bidsname}.* already exists and will be deleted -- check your results carefully!")
if (outfolder/bidsname).with_suffix('.json').is_file():
LOGGER.warning(f"{outfolder/bidsname}.* already exists and will be deleted -- check your results carefully!")
for ext in ('.nii.gz', '.nii', '.json', '.tsv', '.tsv.gz'):
(outfolder / bidsname).with_suffix(ext).unlink(missing_ok=True)
(outfolder/bidsname).with_suffix(ext).unlink(missing_ok=True)

# Convert the source-files in the run folder to nifti's in the BIDS-folder
else:
command = f'{options["command"]} "{source}" -d {outfolder / Path(bidsname).with_suffix(".nii.gz")}'
command = f'{options["command"]} "{source}" -d {outfolder/Path(bidsname).with_suffix(".nii.gz")}'
# pass in data added via bidseditor/bidsmap
if len(run.get('meta', {})) > 0:
command += ' --kwargs '
Expand Down Expand Up @@ -312,7 +307,7 @@ def bidscoiner_plugin(session: Path, bidsmap: dict, bidsses: Path) -> None:
personals['weight'] = datasource.attributes('PatientWeight')

# Store the collected personals in the participants_table
participants_tsv = bidsfolder / 'participants.tsv'
participants_tsv = bidsfolder/'participants.tsv'
if participants_tsv.is_file():
participants_table = pd.read_csv(participants_tsv, sep='\t', dtype=str)
participants_table.set_index(['participant_id'], verify_integrity=True, inplace=True)
Expand Down
7 changes: 1 addition & 6 deletions bidscoin/plugins/spec2nii2bids.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,12 +224,7 @@ def bidscoiner_plugin(session: Path, bidsmap: dict, bidsses: Path) -> 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')
runindex = run['bids'].get('run')
runindex = str(runindex) if runindex else ''
if runindex.startswith('<<') and runindex.endswith('>>'):
bidsname = bids.increment_runindex(outfolder, bidsname)
if runindex == '<<>>' and 'run-2' in bidsname:
bids.add_run1_keyval(outfolder, bidsname, scans_table, bidsses)
bidsname = bids.increment_runindex(outfolder, bidsname, run, scans_table)
jsonfile = (outfolder/bidsname).with_suffix('.json')

# Check if the bidsname is valid
Expand Down
60 changes: 28 additions & 32 deletions tests/test_bids.py
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import tempfile
from unittest.mock import call, patch, Mock

import pandas as pd
import pytest
import shutil
import re
import json
import ruamel.yaml.comments
from unittest.mock import call, patch, Mock
from pathlib import Path
from nibabel.testing import data_path
from pydicom.data import get_testdata_file
Expand Down Expand Up @@ -234,14 +233,13 @@ def test_delete_run(test_bidsmap):
@patch.object(Path, 'rename')
def test_add_run1_keyval(rename_mock: Mock, glob_mock: Mock):
input_bidsname = 'sub-01_run-2_T1w'
old_bidsname = 'sub-01_T1w'
new_bidsname = 'sub-01_run-1_T1w'
outfolder = Path.home() / 'mock-dataset' / 'sub-01' / 'anat'
bidsses = Path.home() / 'mock-dataset' / 'sub-01'
old_bidsname = 'sub-01_T1w'
new_bidsname = 'sub-01_run-1_T1w'
outfolder = Path.home()/'mock-dataset'/'sub-01'/'anat'
glob_mock.return_value = [
(outfolder / old_bidsname).with_suffix('.nii.gz'),
(outfolder / old_bidsname).with_suffix('.json'),
]
(outfolder/old_bidsname).with_suffix('.nii.gz'),
(outfolder/old_bidsname).with_suffix('.json'),
]
scans_data = {
'filename': ['anat/sub-01_rec-norm_T1w.nii.gz', 'anat/sub-01_T1w.nii.gz'],
'acq_time': ['mock-acq1', 'mock-acq2'],
Expand All @@ -250,65 +248,63 @@ def test_add_run1_keyval(rename_mock: Mock, glob_mock: Mock):
"filename": ['anat/sub-01_rec-norm_T1w.nii.gz', 'anat/sub-01_run-1_T1w.nii.gz'],
"acq_time": ['mock-acq1', 'mock-acq2'],
}
scans_table = pd.DataFrame(scans_data)
scans_table.set_index('filename', inplace=True)
result_scans_table = pd.DataFrame(result_scans_data)
result_scans_table.set_index('filename', inplace=True)
scans_table = pd.DataFrame(scans_data).set_index('filename')
result_scans_table = pd.DataFrame(result_scans_data).set_index('filename')

bids.add_run1_keyval(outfolder, input_bidsname, scans_table, bidsses)
bids.increment_runindex(outfolder, input_bidsname, {'bids': {'run': '<<>>'}}, scans_table)

expected_calls = [
call(outfolder / f'{new_bidsname}.nii.gz'),
call(outfolder / f'{new_bidsname}.json'),
call(outfolder/f'{new_bidsname}.nii.gz'),
call(outfolder/f'{new_bidsname}.json'),
]
rename_mock.assert_has_calls(expected_calls)
assert result_scans_table.equals(scans_table)


@patch.object(Path, 'glob', autospec=True, return_value=[])
def test_increment_runindex_no_run1(_):
bidsname = 'sub-01_run-1_T1w'
bidsname = 'sub-01_run-1_T1w'
expected_bidsname = 'sub-01_run-1_T1w'
bidsfolder = Path.home() / 'mock-dataset' / 'sub-01' / 'anat'
result_bidsname = bids.increment_runindex(bidsfolder, bidsname)
bidsfolder = Path.home()/'mock-dataset'/'sub-01'/'anat'
result_bidsname = bids.increment_runindex(bidsfolder, bidsname, {'bids': {'run': '<<>>'}})
assert result_bidsname == expected_bidsname


@patch.object(Path, 'glob', autospec=True, side_effect=[['sub-01_run-1_T1w'], ['sub-01_run-2_T1w'], []])
def test_increment_runindex_run1_run2_exists(_):
bidsname = 'sub-01_run-1_T1w'
bidsname = 'sub-01_run-1_T1w'
expected_bidsname = 'sub-01_run-3_T1w'
bidsfolder = Path.home() / 'mock-dataset' / 'sub-01' / 'anat'
result_bidsname = bids.increment_runindex(bidsfolder, bidsname)
bidsfolder = Path.home()/'mock-dataset'/'sub-01'/'anat'
result_bidsname = bids.increment_runindex(bidsfolder, bidsname, {'bids': {'run': '<<>>'}})
assert result_bidsname == expected_bidsname


@patch.object(Path, 'glob', autospec=True, return_value=[])
def test_increment_runindex_empty_dynamic_finds_run1(_):
bidsname = 'sub-01_T1w' # runindex is <<>> so no run is added to bidsname
bidsname = 'sub-01_T1w' # runindex is <<>> so no run is added to bidsname
expected_bidsname = 'sub-01_T1w'
bidsfolder = Path.home() / 'mock-dataset' / 'sub-01' / 'anat'
result_bidsname = bids.increment_runindex(bidsfolder, bidsname)
bidsfolder = Path.home()/'mock-dataset'/'sub-01'/'anat'
result_bidsname = bids.increment_runindex(bidsfolder, bidsname, {'bids': {'run': '<<>>'}})
assert result_bidsname == expected_bidsname


@patch.object(Path, 'glob', autospec=True)
def test_increment_runindex_empty_dynamic_finds_run2(mock_glob):
bidsname = 'sub-01_T1w' # runindex is <<>> so no run is added to bidsname
bidsname = 'sub-01_T1w' # runindex is <<>> so no run is added to bidsname
expected_bidsname = 'sub-01_run-2_T1w'
bidsfolder = Path.home() / 'mock-dataset' / 'sub-01' / 'anat'
bidsfolder = Path.home()/'mock-dataset'/'sub-01'/'anat'
# [no run1 (default without run keyval), sub-01_T1w exists (run1), no run2]
mock_glob.side_effect = [[], ['sub-01_T1w'], []]
result_bidsname = bids.increment_runindex(bidsfolder, bidsname)
mock_glob.side_effect = [[], ['sub-01_T1w'], [], []]
result_bidsname = bids.increment_runindex(bidsfolder, bidsname, {'bids': {'run': '<<>>'}})
assert result_bidsname == expected_bidsname


@patch.object(Path, 'glob', autospec=True)
def test_increment_runindex_empty_dynamic_finds_run3(mock_glob):
bidsname = 'sub-01_T1w' # runindex is <<>> so no run is added to bidsname
bidsname = 'sub-01_T1w' # runindex is <<>> so no run is added to bidsname
expected_bidsname = 'sub-01_run-3_T1w'
bidsfolder = Path.home() / 'mock-dataset' / 'sub-01' / 'anat'
bidsfolder = Path.home()/'mock-dataset'/'sub-01'/'anat'
# [run1 exists, run1 exists, run2 exists, no run3)
mock_glob.side_effect = [['sub-01_run-1_T1w'], ['sub-01_run-1_T1w'], ['sub-01_run-2_T1w'], []]
result_bidsname = bids.increment_runindex(bidsfolder, bidsname)
result_bidsname = bids.increment_runindex(bidsfolder, bidsname, {'bids': {'run': '<<>>'}})
assert result_bidsname == expected_bidsname

0 comments on commit 3c9b896

Please sign in to comment.