Merge pull request #195 from DominikaZ/run-naming

Add dynamic run index <<>> option

bidscoin/bids.py

@@ -20,6 +20,7 @@
 from pathlib import Path
 from typing import Union, List, Tuple
 from nibabel.parrec import parse_PAR_header
+from pandas import DataFrame
 from pydicom import dcmread, fileset, datadict
 from importlib.util import find_spec
 if find_spec('bidscoin') is None:
@@ -1292,7 +1293,7 @@ def cleanup_value(label: str) -> str:
     :return:        The cleaned-up / BIDS-valid label
     """
 
-    if label is None:
+    if label is None or label == '':
         return ''
     if not isinstance(label, str):
         return label
@@ -1385,7 +1386,7 @@ def get_run(bidsmap: dict, datatype: str, suffix_idx: Union[int, str], datasourc
 
             # Replace the dynamic bids values, except the dynamic run-index (e.g. <<1>>)
             for bidskey, bidsvalue in run['bids'].items():
-                if bidskey == 'run' and bidsvalue and bidsvalue.replace('<','').replace('>','').isdecimal():
+                if bidskey == 'run' and bidsvalue and (bidsvalue.replace('<','').replace('>','').isdecimal() or bidsvalue == '<<>>'):
                     run_['bids'][bidskey] = bidsvalue
                 else:
                     run_['bids'][bidskey] = datasource.dynamicvalue(bidsvalue)
@@ -1678,7 +1679,7 @@ def get_matching_run(datasource: DataSource, bidsmap: dict, runtime=False) -> Tu
             for bidskey, bidsvalue in run['bids'].items():
 
                 # Replace the dynamic bids values, except the dynamic run-index (e.g. <<1>>)
-                if bidskey == 'run' and bidsvalue and bidsvalue.replace('<','').replace('>','').isdecimal():
+                if bidskey == 'run' and bidsvalue and (bidsvalue.replace('<','').replace('>','').isdecimal() or bidsvalue == '<<>>'):
                     run_['bids'][bidskey] = bidsvalue
                 else:
                     run_['bids'][bidskey] = datasource.dynamicvalue(bidsvalue, runtime=runtime)
@@ -1749,11 +1750,11 @@ def get_bidsname(subid: str, sesid: str, run: dict, validkeys: bool, runtime: bo
             bidsvalue = ''
         if isinstance(bidsvalue, list):
             bidsvalue = bidsvalue[bidsvalue[-1]]                                # Get the selected item
-        elif runtime and not (entitykey=='run' and bidsvalue.replace('<','').replace('>','').isdecimal()):
+        elif runtime and not (entitykey=='run' and (bidsvalue.replace('<','').replace('>','').isdecimal() or bidsvalue == '<<>>')):
             bidsvalue = run['datasource'].dynamicvalue(bidsvalue, cleanup=True, runtime=runtime)
+        if cleanup:
+            bidsvalue = cleanup_value(bidsvalue)
         if bidsvalue:
-            if cleanup:
-                bidsvalue = cleanup_value(bidsvalue)
             bidsname = f"{bidsname}_{entitykey}-{bidsvalue}"                    # Append the key-value data to the bidsname
     suffix = run['bids'].get('suffix')
     if runtime:
@@ -1869,25 +1870,52 @@ 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:
+    """
+    Adds run-1 key to files with bidsname that don't have run index. Updates scans 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
+    """
+    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)
+
+
 def increment_runindex(bidsfolder: Path, bidsname: str, ext: str='.*') -> Union[Path, str]:
     """
-    Checks if a file with the same the bidsname already exists in the folder and then increments the runindex (if any)
-    until no such file is found
+    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)
+        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)):
 
         runindex = get_bidsvalue(bidsname, 'run')
-        if runindex:
-            bidsname = get_bidsvalue(bidsname, 'run', str(int(runindex) + 1))
+        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:
-            LOGGER.error(f"Could not increment run-index in: {bidsfolder/bidsname}")
-            break
+            bidsname = get_bidsvalue(bidsname, 'run', str(int(runindex) + 1))
 
     return bidsname
 

bidscoin/plugins/dcm2niix2bids.py

@@ -258,6 +258,8 @@ def bidscoiner_plugin(session: Path, bidsmap: dict, bidsses: Path) -> None:
         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)
         jsonfiles  = [(outfolder/bidsname).with_suffix('.json')]     # List -> Collect the associated json-files (for updating them later) -- possibly > 1
 
         # Check if the bidsname is valid
@@ -409,6 +411,8 @@ def bidscoiner_plugin(session: Path, bidsmap: dict, bidsses: Path) -> None:
                 # 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)
                 newbidsfile = outfolder/newbidsname
                 LOGGER.verbose(f"Found dcm2niix {postfixes} postfixes, renaming\n{dcm2niixfile} ->\n{newbidsfile}")
                 if newbidsfile.is_file():

bidscoin/plugins/nibabel2bids.py

@@ -216,6 +216,8 @@ def bidscoiner_plugin(session: Path, bidsmap: dict, bidsses: Path) -> None:
         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)
 
         # Check if the bidsname is valid

bidscoin/plugins/pet2bids.py

@@ -259,6 +259,8 @@ def bidscoiner_plugin(session: Path, bidsmap: dict, bidsses: Path) -> None:
             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)
 
             # Check if the bidsname is valid
             bidstest = (Path('/') / subid / sesid / datasource.datatype / bidsname).with_suffix('.json').as_posix()

bidscoin/plugins/spec2nii2bids.py

@@ -12,6 +12,7 @@
 from bids_validator import BIDSValidator
 from pathlib import Path
 from bidscoin import bcoin, bids
+from bidscoin.bids import add_run1_keyval
 
 LOGGER = logging.getLogger(__name__)
 
@@ -228,6 +229,8 @@ def bidscoiner_plugin(session: Path, bidsmap: dict, bidsses: Path) -> None:
         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:
+                add_run1_keyval(outfolder, bidsname, scans_table, bidsses)
         jsonfile   = (outfolder/bidsname).with_suffix('.json')
 
         # Check if the bidsname is valid

tests/test_bids.py

@@ -1,4 +1,7 @@
 import tempfile
+from unittest.mock import call, patch, Mock
+
+import pandas as pd
 import pytest
 import shutil
 import re
@@ -225,3 +228,87 @@ def test_delete_run(test_bidsmap):
         written_bidsmap, _ = bids.load_bidsmap(_)
         deleted_run = bids.find_run(written_bidsmap, anat_provenance)
         assert deleted_run is None
+
+
+@patch.object(Path, 'glob')
+@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'
+    glob_mock.return_value = [
+            (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'],
+    }
+    result_scans_data = {
+        "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)
+
+    bids.add_run1_keyval(outfolder, input_bidsname, scans_table, bidsses)
+
+    expected_calls = [
+        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'
+    expected_bidsname = 'sub-01_run-1_T1w'
+    bidsfolder = Path.home() / 'mock-dataset' / 'sub-01' / 'anat'
+    result_bidsname = bids.increment_runindex(bidsfolder, bidsname)
+    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'
+    expected_bidsname = 'sub-01_run-3_T1w'
+    bidsfolder = Path.home() / 'mock-dataset' / 'sub-01' / 'anat'
+    result_bidsname = bids.increment_runindex(bidsfolder, bidsname)
+    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
+    expected_bidsname = 'sub-01_T1w'
+    bidsfolder = Path.home() / 'mock-dataset' / 'sub-01' / 'anat'
+    result_bidsname = bids.increment_runindex(bidsfolder, bidsname)
+    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
+    expected_bidsname = 'sub-01_run-2_T1w'
+    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)
+    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
+    expected_bidsname = 'sub-01_run-3_T1w'
+    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)
+    assert result_bidsname == expected_bidsname