Skip to content

Commit

Permalink
Merge remote-tracking branch 'upstream/master' into enh/complex_realimag
Browse files Browse the repository at this point in the history
  • Loading branch information
bpinsard committed Oct 31, 2024
2 parents 152fc2d + 2eb5291 commit 19666a9
Show file tree
Hide file tree
Showing 15 changed files with 399 additions and 73 deletions.
3 changes: 3 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ jobs:
- '3.9'
- '3.10'
- '3.11'
- '3.12'
# Seems needs work in traits: https://github.com/nipy/heudiconv/pull/799#issuecomment-2447298795
# - '3.13'
steps:
- name: Check out repository
uses: actions/checkout@v4
Expand Down
33 changes: 33 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,36 @@
# v1.3.1 (Fri Oct 25 2024)

#### 🐛 Bug Fix

- Fix assignment of sensitive git-annex metadata data via glob patterns (regression introduced by #739) [#793](https://github.com/nipy/heudiconv/pull/793) ([@bpinsard](https://github.com/bpinsard))

#### Authors: 1

- Basile ([@bpinsard](https://github.com/bpinsard))

---

# v1.3.0 (Wed Oct 02 2024)

#### 🚀 Enhancement

- timezone aware [#780](https://github.com/nipy/heudiconv/pull/780) ([@AlanKuurstra](https://github.com/AlanKuurstra) [@yarikoptic](https://github.com/yarikoptic))

#### 🐛 Bug Fix

- BF(workaround): if heuristic provided just a string and not list of types -- make it into a tuple [#787](https://github.com/nipy/heudiconv/pull/787) ([@yarikoptic](https://github.com/yarikoptic))
- Refactor create_seqinfo tiny bit to avoid duplication and add logging; and in tests to reuse list of dicom paths [#785](https://github.com/nipy/heudiconv/pull/785) ([@yarikoptic](https://github.com/yarikoptic))
- extract sequence_name from PulseSequenceName on Siemens XA** data [#753](https://github.com/nipy/heudiconv/pull/753) ([@bpinsard](https://github.com/bpinsard))
- Just INFO not WARNING if heuristic is missing intotoids [#784](https://github.com/nipy/heudiconv/pull/784) ([@yarikoptic](https://github.com/yarikoptic))

#### Authors: 3

- [@AlanKuurstra](https://github.com/AlanKuurstra)
- Basile ([@bpinsard](https://github.com/bpinsard))
- Yaroslav Halchenko ([@yarikoptic](https://github.com/yarikoptic))

---

# v1.2.0 (Fri Sep 13 2024)

#### 🚀 Enhancement
Expand Down
14 changes: 7 additions & 7 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Generated by Neurodocker and Reproenv.

FROM neurodebian:bullseye
ENV PATH="/opt/dcm2niix-v1.0.20220720/bin:$PATH"
FROM neurodebian:bookworm
ENV PATH="/opt/dcm2niix-v1.0.20240202/bin:$PATH"
RUN apt-get update -qq \
&& apt-get install -y -q --no-install-recommends \
ca-certificates \
Expand All @@ -16,10 +16,10 @@ RUN apt-get update -qq \
&& git clone https://github.com/rordenlab/dcm2niix /tmp/dcm2niix \
&& cd /tmp/dcm2niix \
&& git fetch --tags \
&& git checkout v1.0.20220720 \
&& git checkout v1.0.20240202 \
&& mkdir /tmp/dcm2niix/build \
&& cd /tmp/dcm2niix/build \
&& cmake -DZLIB_IMPLEMENTATION=Cloudflare -DUSE_JPEGLS=ON -DUSE_OPENJPEG=ON -DCMAKE_INSTALL_PREFIX:PATH=/opt/dcm2niix-v1.0.20220720 .. \
&& cmake -DZLIB_IMPLEMENTATION=Cloudflare -DUSE_JPEGLS=ON -DUSE_OPENJPEG=ON -DCMAKE_INSTALL_PREFIX:PATH=/opt/dcm2niix-v1.0.20240202 .. \
&& make -j1 \
&& make install \
&& rm -rf /tmp/dcm2niix
Expand Down Expand Up @@ -87,19 +87,19 @@ RUN printf '{ \
{ \
"name": "from_", \
"kwds": { \
"base_image": "neurodebian:bullseye" \
"base_image": "neurodebian:bookworm" \
} \
}, \
{ \
"name": "env", \
"kwds": { \
"PATH": "/opt/dcm2niix-v1.0.20220720/bin:$PATH" \
"PATH": "/opt/dcm2niix-v1.0.20240202/bin:$PATH" \
} \
}, \
{ \
"name": "run", \
"kwds": { \
"command": "apt-get update -qq\\napt-get install -y -q --no-install-recommends \\\\\\n ca-certificates \\\\\\n cmake \\\\\\n g++ \\\\\\n gcc \\\\\\n git \\\\\\n make \\\\\\n pigz \\\\\\n zlib1g-dev\\nrm -rf /var/lib/apt/lists/*\\ngit clone https://github.com/rordenlab/dcm2niix /tmp/dcm2niix\\ncd /tmp/dcm2niix\\ngit fetch --tags\\ngit checkout v1.0.20220720\\nmkdir /tmp/dcm2niix/build\\ncd /tmp/dcm2niix/build\\ncmake -DZLIB_IMPLEMENTATION=Cloudflare -DUSE_JPEGLS=ON -DUSE_OPENJPEG=ON -DCMAKE_INSTALL_PREFIX:PATH=/opt/dcm2niix-v1.0.20220720 ..\\nmake -j1\\nmake install\\nrm -rf /tmp/dcm2niix" \
"command": "apt-get update -qq\\napt-get install -y -q --no-install-recommends \\\\\\n ca-certificates \\\\\\n cmake \\\\\\n g++ \\\\\\n gcc \\\\\\n git \\\\\\n make \\\\\\n pigz \\\\\\n zlib1g-dev\\nrm -rf /var/lib/apt/lists/*\\ngit clone https://github.com/rordenlab/dcm2niix /tmp/dcm2niix\\ncd /tmp/dcm2niix\\ngit fetch --tags\\ngit checkout v1.0.20240202\\nmkdir /tmp/dcm2niix/build\\ncd /tmp/dcm2niix/build\\ncmake -DZLIB_IMPLEMENTATION=Cloudflare -DUSE_JPEGLS=ON -DUSE_OPENJPEG=ON -DCMAKE_INSTALL_PREFIX:PATH=/opt/dcm2niix-v1.0.20240202 ..\\nmake -j1\\nmake install\\nrm -rf /tmp/dcm2niix" \
} \
}, \
{ \
Expand Down
4 changes: 4 additions & 0 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@
:target: https://repology.org/project/python:heudiconv/versions
:alt: PyPI

.. image:: https://img.shields.io/badge/RRID-SCR__017427-blue
:target: https://identifiers.org/RRID:SCR_017427
:alt: RRID

About
-----

Expand Down
10 changes: 4 additions & 6 deletions heudiconv/bids.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
remove_suffix,
save_json,
set_readonly,
strptime_micr,
strptime_bids,
update_json,
)

Expand Down Expand Up @@ -952,18 +952,16 @@ def select_fmap_from_compatible_groups(
k for k, v in acq_times_fmaps.items() if v == first_acq_time
][0]
elif criterion == "Closest":
json_acq_time = strptime_micr(
json_acq_time = strptime_bids(
acq_times[
# remove session folder and '.json', add '.nii.gz':
remove_suffix(remove_prefix(json_file, sess_folder + op.sep), ".json")
+ ".nii.gz"
],
"%Y-%m-%dT%H:%M:%S[.%f]",
]
)
# differences in acquisition time (abs value):
diff_fmaps_acq_times = {
k: abs(strptime_micr(v, "%Y-%m-%dT%H:%M:%S[.%f]") - json_acq_time)
for k, v in acq_times_fmaps.items()
k: abs(strptime_bids(v) - json_acq_time) for k, v in acq_times_fmaps.items()
}
min_diff_acq_times = sorted(diff_fmaps_acq_times.values())[0]
selected_fmap_key = [
Expand Down
9 changes: 9 additions & 0 deletions heudiconv/convert.py
Original file line number Diff line number Diff line change
Expand Up @@ -579,6 +579,15 @@ def convert(

for item in items:
prefix, outtypes, item_dicoms = item
if isinstance(outtypes, str): # type: ignore[unreachable]
lgr.warning( # type: ignore[unreachable]
"Provided output types %r of type 'str' instead "
"of a tuple for prefix %r. Likely need to fix-up your heuristic. "
"Meanwhile we are 'manually' converting to 'tuple'",
outtypes,
prefix,
)
outtypes = (outtypes,)
prefix_dirname = op.dirname(prefix)
outname_bids = prefix + ".json"
bids_outfiles = []
Expand Down
48 changes: 25 additions & 23 deletions heudiconv/dicoms.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@
get_typed_attr,
load_json,
set_readonly,
strptime_micr,
strptime_dcm_da_tm,
strptime_dcm_dt,
)

if TYPE_CHECKING:
Expand Down Expand Up @@ -94,15 +95,19 @@ def create_seqinfo(
series_desc = get_typed_attr(dcminfo, "SeriesDescription", str, "")
protocol_name = get_typed_attr(dcminfo, "ProtocolName", str, "")

if dcminfo.get([0x18, 0x24]):
# GE and Philips
sequence_name = dcminfo[0x18, 0x24].value
elif dcminfo.get([0x19, 0x109C]):
# Siemens
sequence_name = dcminfo[0x19, 0x109C].value
elif dcminfo.get([0x18, 0x9005]):
# Siemens XA
sequence_name = dcminfo[0x18, 0x9005].value
for k, m in (
([0x18, 0x24], "GE and Philips"),
([0x19, 0x109C], "Siemens"),
([0x18, 0x9005], "Siemens XA"),
):
if v := dcminfo.get(k):
sequence_name = v.value
lgr.debug(
"Identified sequence name as %s coming from the %r family of MR scanners",
sequence_name,
m,
)
break
else:
sequence_name = ""

Expand Down Expand Up @@ -544,19 +549,16 @@ def get_datetime_from_dcm(dcm_data: dcm.FileDataset) -> Optional[datetime.dateti
3. SeriesDate & SeriesTime (0008,0021); (0008,0031)
"""
acq_date = dcm_data.get("AcquisitionDate", "").strip()
acq_time = dcm_data.get("AcquisitionTime", "").strip()
if acq_date and acq_time:
return strptime_micr(acq_date + acq_time, "%Y%m%d%H%M%S[.%f]")

acq_dt = dcm_data.get("AcquisitionDateTime", "").strip()
if acq_dt:
return strptime_micr(acq_dt, "%Y%m%d%H%M%S[.%f]")

series_date = dcm_data.get("SeriesDate", "").strip()
series_time = dcm_data.get("SeriesTime", "").strip()
if series_date and series_time:
return strptime_micr(series_date + series_time, "%Y%m%d%H%M%S[.%f]")

def check_tag(x: str) -> bool:
return x in dcm_data and dcm_data[x].value.strip()

if check_tag("AcquisitionDate") and check_tag("AcquisitionTime"):
return strptime_dcm_da_tm(dcm_data, "AcquisitionDate", "AcquisitionTime")
if check_tag("AcquisitionDateTime"):
return strptime_dcm_dt(dcm_data, "AcquisitionDateTime")
if check_tag("SeriesDate") and check_tag("SeriesTime"):
return strptime_dcm_da_tm(dcm_data, "SeriesDate", "SeriesTime")
return None


Expand Down
6 changes: 3 additions & 3 deletions heudiconv/external/dlad.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,12 +156,12 @@ def add_to_datalad(

# Provide metadata for sensitive information
sensitive_patterns = [
"sourcedata",
"sourcedata/**",
"*_scans.tsv", # top level
"*/*_scans.tsv", # within subj
"*/*/*_scans.tsv", # within sess/subj
"*/anat", # within subj
"*/*/anat", # within ses/subj
"*/anat/*", # within subj
"*/*/anat/*", # within ses/subj
]
for sp in sensitive_patterns:
mark_sensitive(ds, sp, annexed_files)
Expand Down
25 changes: 22 additions & 3 deletions heudiconv/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import os.path as op
import re
import shutil
import sys
from types import ModuleType
from typing import Optional

Expand All @@ -22,7 +23,18 @@

_VCS_REGEX = r"%s\.(?:git|gitattributes|svn|bzr|hg)(?:%s|$)" % (op.sep, op.sep)

_UNPACK_FORMATS = tuple(sum((x[1] for x in shutil.get_unpack_formats()), []))

def _get_unpack_formats() -> dict[str, bool]:
"""For each extension return if it is a tar"""
out = {}
for _, exts, d in shutil.get_unpack_formats():
for e in exts:
out[e] = bool(re.search(r"\btar\b", d.lower()))
return out


_UNPACK_FORMATS = _get_unpack_formats()
_TAR_UNPACK_FORMATS = tuple(k for k, is_tar in _UNPACK_FORMATS.items() if is_tar)


@docstring_parameter(_VCS_REGEX)
Expand Down Expand Up @@ -114,7 +126,7 @@ def get_extracted_dicoms(fl: Iterable[str]) -> ItemsView[Optional[str], list[str

# needs sorting to keep the generated "session" label deterministic
for _, t in enumerate(sorted(fl)):
if not t.endswith(_UNPACK_FORMATS):
if not t.endswith(tuple(_UNPACK_FORMATS)):
sessions[None].append(t)
continue

Expand All @@ -127,7 +139,14 @@ def get_extracted_dicoms(fl: Iterable[str]) -> ItemsView[Optional[str], list[str

# check content and sanitize permission bits before extraction
os.chmod(tmpdir, mode=0o700)
shutil.unpack_archive(t, extract_dir=tmpdir)
# For tar (only!) starting with 3.12 we should provide filter
# (enforced in 3.14) on how to filter/safe-guard filenames.
kws: dict[str, str] = {}
if sys.version_info >= (3, 12) and t.endswith(_TAR_UNPACK_FORMATS):
# Allow for a user-workaround if would be desired
# see e.g. https://docs.python.org/3.12/library/tarfile.html#extraction-filters
kws["filter"] = os.environ.get("HEUDICONV_TAR_FILTER", "tar")
shutil.unpack_archive(t, extract_dir=tmpdir, **kws) # type: ignore[arg-type]

archive_content = list(find_files(regex=".*", topdir=tmpdir))

Expand Down
25 changes: 8 additions & 17 deletions heudiconv/tests/test_dicoms.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
parse_private_csa_header,
)

from .utils import TESTS_DATA_PATH
from .utils import TEST_DICOM_PATHS, TESTS_DATA_PATH

# Public: Private DICOM tags
DICOM_FIELDS_TO_TEST = {"ProtocolName": "tProtocolName"}
Expand Down Expand Up @@ -180,26 +180,17 @@ def test_get_datetime_from_dcm_wo_dt() -> None:
assert get_datetime_from_dcm(XA30_enhanced_dcm) is None


dicom_test_data = [
(dw.wrapper_from_file(d_file), [d_file], op.basename(d_file))
for d_file in glob(op.join(TESTS_DATA_PATH, "*.dcm"))
]


@pytest.mark.parametrize("mw,series_files,series_id", dicom_test_data)
@pytest.mark.parametrize("dcmfile", TEST_DICOM_PATHS)
def test_create_seqinfo(
mw: dw.Wrapper,
series_files: list[str],
series_id: str,
dcmfile: str,
) -> None:
seqinfo = create_seqinfo(mw, series_files, series_id)
assert seqinfo.sequence_name != ""
pass

mw = dw.wrapper_from_file(dcmfile)
seqinfo = create_seqinfo(mw, [dcmfile], op.basename(dcmfile))
assert seqinfo.sequence_name

def test_get_reproducible_int() -> None:
dcmfile = op.join(TESTS_DATA_PATH, "phantom.dcm")

@pytest.mark.parametrize("dcmfile", TEST_DICOM_PATHS)
def test_get_reproducible_int(dcmfile: str) -> None:
assert type(get_reproducible_int([dcmfile])) is int


Expand Down
13 changes: 13 additions & 0 deletions heudiconv/tests/test_regression.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ def test_conversion(
heuristic,
anon_cmd,
template="sourcedata/sub-{subject}/*/*/*.tgz",
xargs=["--datalad"],
)
runner(args) # run conversion

Expand Down Expand Up @@ -96,6 +97,18 @@ def test_conversion(
for key in keys:
assert orig[key] == conv[key]

# validate sensitive marking
from datalad.api import Dataset

ds = Dataset(outdir)
all_meta = dict(ds.repo.get_metadata("."))
target_rec = {"distribution-restrictions": ["sensitive"]}
for pth, meta in all_meta.items():
if "anat" in pth or "scans.tsv" in pth:
assert meta == target_rec
else:
assert meta == {}


@pytest.mark.skipif(not have_datalad, reason="no datalad")
def test_multiecho(
Expand Down
Loading

0 comments on commit 19666a9

Please sign in to comment.