Skip to content
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

[BF] fix sidecars suggested by Sam #243

Merged
merged 5 commits into from
Jul 21, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions dcm2bids/acquisition.py
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ def dstSidecarData(self, idList):
value = [value]

for val in value:
if isinstance(val, str):
if isinstance(val, str) or isinstance(val, bool):
if val not in idList and key in DEFAULT.keyWithPathsidecar_changes:
logging.warning(f"No id found for '{key}' value '{val}'.")
logging.warning(f"No sidecar changes for field '{key}' "
Expand All @@ -221,6 +221,11 @@ def dstSidecarData(self, idList):
"with this id.")
else:
values.append(idList.get(val, val))
if values[-1] != val:
if isinstance(values[-1], list):
values[-1] = "bids::" + values[-1][0]
else:
values[-1] = "bids::" + values[-1]

# handle if nested list vs str
flat_value_list = []
Expand All @@ -229,10 +234,11 @@ def dstSidecarData(self, idList):
flat_value_list += item
else:
flat_value_list.append(item)

if len(flat_value_list) == 1:
flat_value_list = flat_value_list[0]

data[key] = flat_value_list
data[key] = flat_value_list[0]
else:
data[key] = flat_value_list

return data

Expand Down
6 changes: 4 additions & 2 deletions dcm2bids/dcm2bids_gen.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,9 +155,11 @@ def move(self, acquisition, idList, post_op):
# Populate idList
if '.nii' in ext:
if acquisition.id in idList:
idList[acquisition.id].append(acquisition.dstId + "".join(ext))
idList[acquisition.id].append(os.path.join(acquisition.participant.name,
acquisition.dstId + "".join(ext)))
else:
idList[acquisition.id] = [acquisition.dstId + "".join(ext)]
idList[acquisition.id] = [os.path.join(acquisition.participant.name,
acquisition.dstId + "".join(ext))]

for curr_post_op in post_op:
if acquisition.datatype in curr_post_op['datatype'] or 'any' in curr_post_op['datatype']:
Expand Down
32 changes: 17 additions & 15 deletions docs/how-to/use-advanced-commands.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
# How to use advanced configuration
# Advanced configuration and command
arnaudbore marked this conversation as resolved.
Show resolved Hide resolved

## How to use advanced configuration

These optional configurations can be inserted in the configuration file at the
same level as the `"description"` entry.
Expand Down Expand Up @@ -43,7 +45,7 @@ same level as the `"description"` entry.
}
```

## custom_entities combined with extractors
### custom_entities combined with extractors
arnaudbore marked this conversation as resolved.
Show resolved Hide resolved

default: None

Expand All @@ -57,15 +59,15 @@ entities directly into the final filename. custom_entities can be a list that
combined extractor keys and regular entities. If key is `task` it will
automatically add the field "TaskName" inside the sidecase file.

## search_method
### search_method
arnaudbore marked this conversation as resolved.
Show resolved Hide resolved

default: `"search_method": "fnmatch"`

fnmatch is the behaviour (See criteria) by default and the fall back if this
option is set incorrectly. `re` is the other choice if you want more flexibility
to match criteria.

## dup_method
### dup_method
arnaudbore marked this conversation as resolved.
Show resolved Hide resolved

default: `"dup_method": "run"`

Expand All @@ -77,14 +79,14 @@ customEntities of the other acquisitions. This behavior is a
[heudiconv](https://heudiconv.readthedocs.io/en/latest/changes.html) inspired
feature.

## case_sensitive
### case_sensitive
arnaudbore marked this conversation as resolved.
Show resolved Hide resolved

default: `"case_sensitive": "true"`

If false, comparisons between strings/lists will be not case sensitive. It's
only disabled when used with `"search_method": "fnmatch"`.

## post_op
### post_op
arnaudbore marked this conversation as resolved.
Show resolved Hide resolved

default: `"post_op": []`

Expand Down Expand Up @@ -130,23 +132,23 @@ which datatype is fmap.
Finally, this is a template string and dcm2bids will replace `src_file` and
`dst_file` by the source file (input) and the destination file (output).

## dcm2niixOptions
### dcm2niixOptions
arnaudbore marked this conversation as resolved.
Show resolved Hide resolved

default: `"dcm2niixOptions": "-b y -ba y -z y -f '%3s_%f_%p_%t'"`

Arguments for dcm2niix

## compKeys
### compKeys
arnaudbore marked this conversation as resolved.
Show resolved Hide resolved

default: `"compKeys": ["SeriesNumber", "AcquisitionTime", "SidecarFilename"]`

Acquisitions are sorted using the sidecar data. The default behaviour is to sort
by `SeriesNumber` then by `AcquisitionTime` then by the `SidecarFilename`. You
can change this behaviour setting this key inside the configuration file.

## criteria
### criteria
arnaudbore marked this conversation as resolved.
Show resolved Hide resolved

### Handle multi site filtering
#### Handle multi site filtering

As mentionned in the [first-steps tutorial](../tutorial/first-steps.md),
criteria is the way to filter specific acquisitions. If you work with dicoms
Expand All @@ -160,7 +162,7 @@ feature where for a specific criteria you can get multiple descriptions.
}
```

### Enhanced float/int comparison
#### Enhanced float/int comparison

Criteria can help you filter acquisitions by comparing float/int sidecar.

Expand Down Expand Up @@ -196,9 +198,9 @@ If you want to use btw or btwe you will need to give an ordered list like this.
}
```

# How to use advanced commands
## How to use advanced commands

## dcm2bids advanced options
### dcm2bids advanced options

By now, you should be used to getting the `--help` information before running a
command.
Expand Down Expand Up @@ -245,7 +247,7 @@ command.

```

## --auto_extract_entities
### --auto_extract_entities
arnaudbore marked this conversation as resolved.
Show resolved Hide resolved

This option will automatically try to find 3 entities (task, dir and echo) for
specific datatype/suffix.
Expand Down Expand Up @@ -310,7 +312,7 @@ like this.
:radioactive: You can find more detailed information by looking at the file [`dcm2bids/utils/utils.py`](../dcm2bids/utils/utils/) and
more specifically *`auto_extractors`* and *`auto_entities`* variables.

## --bids_validate
### --bids_validate
arnaudbore marked this conversation as resolved.
Show resolved Hide resolved

By default, dcm2bids will not validate your final BIDS structure. If needed, you
can install
Expand Down
92 changes: 92 additions & 0 deletions tests/data/config_test_sidecar.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
{
"descriptions": [
{
"datatype": "localizer",
"suffix": "localizer",
"criteria": {
"SeriesDescription": "locali*"
},
"sidecar_changes": {
"ProcedureStepDescription": "Modified by dcm2bids"
}
},
{
"id": "T1",
"datatype": "anat",
"suffix": "T1w",
"criteria": {
"SidecarFilename": "*MPRAGE*"
},
"sidecar_changes": {
"new_field": "new value"
}
},
{
"id": "id_dwi",
"datatype": "dwi",
"suffix": "dwi",
"criteria": {
"SeriesDescription": "DTI"
}
},
{
"datatype": "fmap",
"suffix": "fmap",
"custom_entities": "echo-492",
"criteria": {
"EchoNumber": 1,
"EchoTime": 0.00492
},
"sidecar_changes": {
"MTState": true
}
},
{
"datatype": "fmap",
"suffix": "fmap",
"custom_entities": "echo-738",
"criteria": {
"EchoNumber": 2,
"EchoTime": 0.00738,
"ImageType": ["ORIGINAL", "PRIMARY", "M", "ND"]
},
"sidecar_changes": {
"MTState": "false"
}
},
{
"datatype": "dwi",
"suffix": "dwi",
"custom_entities": "desc-fa01",
"criteria": {
"SeriesDescription": "DTI_FA"
},
"sidecar_changes": {
"IntendedFor": [
"id_dwi",
"T1"
],
"Sources": [
"T1",
"id_dwi"
]
}
},
{
"datatype": "dwi",
"suffix": "dwi",
"custom_entities": "desc-trace",
"criteria": {
"SeriesDescription": "DTI_TRACEW"
},
"sidecar_changes": {
"IntendedFor": [
"id_dwi"
],
"Sources": [
"T1"
]
}
}
]
}
89 changes: 79 additions & 10 deletions tests/test_dcm2bids.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,18 +82,20 @@ def test_dcm2bids():
"fmap",
"sub-01_echo-492_fmap.json")
data = load_json(fmapFile)
assert data["IntendedFor"] == [os.path.join("dwi",
"sub-01_dwi.nii.gz"),
os.path.join("anat",
"sub-01_T1w.nii")]
assert data["IntendedFor"] == ["bids::" + os.path.join("sub-01",
"dwi",
"sub-01_dwi.nii.gz"),
"bids::" + os.path.join("sub-01",
"anat",
"sub-01_T1w.nii")]

fmapFile = os.path.join(bids_dir.name,
"sub-01",
"fmap",
"sub-01_echo-738_fmap.json")
data = load_json(fmapFile)
fmapMtime = os.stat(fmapFile).st_mtime
assert data["IntendedFor"] == os.path.join("dwi", "sub-01_dwi.nii.gz")
assert data["IntendedFor"] == "bids::" + os.path.join("sub-01", "dwi", "sub-01_dwi.nii.gz")

data = load_json(
os.path.join(
Expand Down Expand Up @@ -229,16 +231,16 @@ def test_dcm2bids_case_sensitive():
"fmap",
"sub-01_echo-492_fmap.json")
data = load_json(fmapFile)
assert data["IntendedFor"] == [os.path.join("dwi", "sub-01_dwi.nii.gz"),
os.path.join("anat", "sub-01_T1w.nii")]
assert data["IntendedFor"] == ["bids::" + os.path.join("sub-01", "dwi", "sub-01_dwi.nii.gz"),
"bids::" + os.path.join("sub-01", "anat", "sub-01_T1w.nii")]

fmapFile = os.path.join(bids_dir.name,
"sub-01",
"fmap",
"sub-01_echo-738_fmap.json")
data = load_json(fmapFile)
fmapMtime = os.stat(fmapFile).st_mtime
assert data["IntendedFor"] == os.path.join("dwi", "sub-01_dwi.nii.gz")
assert data["IntendedFor"] == "bids::" + os.path.join("sub-01", "dwi", "sub-01_dwi.nii.gz")

data = load_json(
os.path.join(
Expand Down Expand Up @@ -286,8 +288,8 @@ def test_dcm2bids_auto_extract():
data = load_json(epi_file)

assert os.path.exists(epi_file)
assert data["IntendedFor"] == [os.path.join("dwi", "sub-01_dwi.nii.gz"),
os.path.join("anat", "sub-01_T1w.nii")]
assert data["IntendedFor"] == ["bids::" + os.path.join("sub-01", "dwi", "sub-01_dwi.nii.gz"),
"bids::" + os.path.join("sub-01", "anat", "sub-01_T1w.nii")]

func_task = os.path.join(bids_dir.name, "sub-01",
"func",
Expand Down Expand Up @@ -443,3 +445,70 @@ def test_dcm2bids_float():

assert os.path.exists(fmap_file)
assert os.path.exists(t1w_file)


def test_dcm2bids_sidecar():
bids_dir = TemporaryDirectory()

tmp_sub_dir = os.path.join(bids_dir.name, DEFAULT.tmp_dir_name, "sub-01_ses-dev")
shutil.copytree(os.path.join(TEST_DATA_DIR, "sidecars"), tmp_sub_dir)

app = Dcm2BidsGen(TEST_DATA_DIR, "01",
os.path.join(TEST_DATA_DIR, "config_test_sidecar.json"),
bids_dir.name,
session="dev")
app.run()

layout = BIDSLayout(bids_dir.name, validate=False)

# existing field
data = load_json(os.path.join(bids_dir.name,
"sub-01",
"ses-dev",
"localizer",
"sub-01_ses-dev_run-01_localizer.json"))
assert data["ProcedureStepDescription"] == "Modified by dcm2bids"

# new field
data = load_json(os.path.join(bids_dir.name,
"sub-01",
"ses-dev",
"anat",
"sub-01_ses-dev_T1w.json"))
assert data["new_field"] == "new value"

# boolean value
data = load_json(os.path.join(bids_dir.name,
"sub-01",
"ses-dev",
"fmap",
"sub-01_ses-dev_echo-492_fmap.json"))
assert data["MTState"] == True

# boolean value if input as a string
data = load_json(os.path.join(bids_dir.name,
"sub-01",
"ses-dev",
"fmap",
"sub-01_ses-dev_echo-738_fmap.json"))
assert data["MTState"] == "false"

# list with > 1 items
data = load_json(os.path.join(bids_dir.name,
"sub-01",
"ses-dev",
"dwi",
"sub-01_ses-dev_desc-fa01_dwi.json"))
assert data["IntendedFor"] == ["bids::" + os.path.join("sub-01", "ses-dev", "dwi", "sub-01_ses-dev_dwi.nii.gz"),
"bids::" + os.path.join("sub-01", "ses-dev", "anat", "sub-01_ses-dev_T1w.nii")]
assert data["Sources"] == ["bids::" + os.path.join("sub-01", "ses-dev", "anat", "sub-01_ses-dev_T1w.nii"),
"bids::" + os.path.join("sub-01", "ses-dev", "dwi", "sub-01_ses-dev_dwi.nii.gz")]

# list with 1 item
data = load_json(os.path.join(bids_dir.name,
"sub-01",
"ses-dev",
"dwi",
"sub-01_ses-dev_desc-trace_dwi.json"))
assert data["IntendedFor"] == "bids::" + os.path.join("sub-01", "ses-dev", "dwi", "sub-01_ses-dev_dwi.nii.gz")
assert data["Sources"] == "bids::" + os.path.join("sub-01", "ses-dev", "anat", "sub-01_ses-dev_T1w.nii")