Skip to content

[FIX] remove XA partial volumes, generated by dcm2niix as derived data #817

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
79 changes: 68 additions & 11 deletions heudiconv/convert.py
Original file line number Diff line number Diff line change
Expand Up @@ -851,6 +851,57 @@
return eg, prov_file


def filter_partial_volumes(
nii_files: list[str],
bids_files: list[str],
bids_metas: list[dict[str, Any]],
):
"""filter interrupted 4D scans volumes with missing slices on XA: see dcm2niix #742

Parameters
----------
nii_files : list[str]
converted nifti filepaths
bids_files: list[str]
converted BIDS json filepaths
bids_metas : list[dict[str, Any]]
list of metadata dict loaded from BIDS json files

Returns
-------
nii_files
filtered niftis
bids_files
filtered BIDS jsons
bids_metas
filtered BIDS metadata

"""
partial_volumes = [not metadata.get("RawImage", True) for metadata in bids_metas]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so, a partial volume is iff RawImage = False? i.e. is every volume with RawImage = False is a partial volume?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the way that it's implemented in dcm2niix for now, but as mentionned above I am not sure if that BIDS tag is used for other cases such as true derived data.

no_partial_volumes = not any(partial_volumes) or all(partial_volumes)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, my had hurts a little trying to do this bool and need for that or. Wouldn't it make it easier to talk in terms of

Suggested change
no_partial_volumes = not any(partial_volumes) or all(partial_volumes)
has_partial_volumes = any(partial_volumes)

? (and then flip if below) or it wouldn't be sufficient?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if "RawImage":False is also present for other derived data, so I tried to narrow down the case where a series produced niftis with both False and not present (or True if ever).
There might still be derived data that we want to convert.


if no_partial_volumes:
return nii_files, bids_files, bids_metas
else:
new_nii_files, new_bids_files, new_bids_metas = [], [], []
for fl, bids_file, bids_meta, is_pv in zip(

Check warning on line 887 in heudiconv/convert.py

View check run for this annotation

Codecov / codecov/patch

heudiconv/convert.py#L886-L887

Added lines #L886 - L887 were not covered by tests
nii_files, bids_files, bids_metas, partial_volumes
):
if is_pv:

Check warning on line 890 in heudiconv/convert.py

View check run for this annotation

Codecov / codecov/patch

heudiconv/convert.py#L890

Added line #L890 was not covered by tests
# remove partial volume
os.remove(fl)
os.remove(bids_file)
lgr.warning(f"dropped {fl} partial volume from interrupted series")

Check warning on line 894 in heudiconv/convert.py

View check run for this annotation

Codecov / codecov/patch

heudiconv/convert.py#L892-L894

Added lines #L892 - L894 were not covered by tests
else:
new_nii_files.append(fl)
new_bids_files.append(bids_file)
new_bids_metas.append(bids_meta)
print(bids_file)
if len(new_nii_files) == 1:
return new_nii_files[0], new_bids_files[0], new_bids_metas[0]
return new_nii_files, new_bids_files, new_bids_metas

Check warning on line 902 in heudiconv/convert.py

View check run for this annotation

Codecov / codecov/patch

heudiconv/convert.py#L896-L902

Added lines #L896 - L902 were not covered by tests


def save_converted_files(
res: Node,
item_dicoms: list[str],
Expand Down Expand Up @@ -887,6 +938,7 @@

bids_outfiles: list[str] = []
res_files = res.outputs.converted_files
bids_files = res.outputs.bids

if not len(res_files):
lgr.debug("DICOMs {} were not converted".format(item_dicoms))
Expand Down Expand Up @@ -922,6 +974,21 @@
# we should provide specific handling for fmap,
# dwi etc which might spit out multiple files

# Also copy BIDS files although they might need to
# be merged/postprocessed later
bids_files = (
sorted(bids_files)
if len(bids_files) == len(res_files)
else [None] * len(res_files)
)
# preload since will be used in multiple spots
bids_metas = [load_json(b) for b in bids_files if b]

res_files, bids_files, bids_metas = filter_partial_volumes(
res_files, bids_files, bids_metas
)

if isinstance(res_files, list):
suffixes = (
[str(i + 1) for i in range(len(res_files))]
if (bids_options is not None)
Expand All @@ -938,16 +1005,6 @@
)
suffixes = [str(-i - 1) for i in range(len(res_files))]

# Also copy BIDS files although they might need to
# be merged/postprocessed later
bids_files = (
sorted(res.outputs.bids)
if len(res.outputs.bids) == len(res_files)
else [None] * len(res_files)
)
# preload since will be used in multiple spots
bids_metas = [load_json(b) for b in bids_files if b]

### Do we have a multi-echo series? ###
# Some Siemens sequences (e.g. CMRR's MB-EPI) set the label 'TE1',
# 'TE2', etc. in the 'ImageType' field. However, other seqs do not
Expand Down Expand Up @@ -1040,7 +1097,7 @@
safe_movefile(res_files, outname, overwrite)
if isdefined(res.outputs.bids):
try:
safe_movefile(res.outputs.bids, outname_bids, overwrite)
safe_movefile(bids_files, outname_bids, overwrite)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would need to double-check on what is the effect here and why e.g. we did not use bids_files to start with here, or why checking isdefined for res.output.bids and not bids_files to be defined...

bids_outfiles.append(outname_bids)
except TypeError: ##catch lists
raise TypeError("Multiple BIDS sidecars detected.")
Expand Down
Binary file not shown.
Binary file not shown.
Loading