From 44124bd7722f96aea3b17b469cdacf235c455a6c Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Tue, 19 Jul 2022 16:10:08 -0400 Subject: [PATCH 01/18] ENH: custom_seqinfo - provide a way for heuristics to extract/add arbitrary value Just a draft implementation --- heudiconv/convert.py | 3 +++ heudiconv/dicoms.py | 29 +++++++++++++++++++++++------ heudiconv/utils.py | 2 ++ 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/heudiconv/convert.py b/heudiconv/convert.py index 5f233fc6..40a575e7 100644 --- a/heudiconv/convert.py +++ b/heudiconv/convert.py @@ -221,6 +221,9 @@ def prep_conversion( dcmfilter=getattr(heuristic, "filter_dicom", None), flatten=True, custom_grouping=getattr(heuristic, "grouping", None), + # callable which will be provided dcminfo and returned + # structure extend seqinfo + custom_seqinfo = getattr(heuristic, 'custom_seqinfo', None), ) elif seqinfo is None: raise ValueError("Neither 'dicoms' nor 'seqinfo' is given") diff --git a/heudiconv/dicoms.py b/heudiconv/dicoms.py index ef51086a..7cdade8c 100644 --- a/heudiconv/dicoms.py +++ b/heudiconv/dicoms.py @@ -9,7 +9,8 @@ from pathlib import Path import sys import tarfile -from typing import TYPE_CHECKING, Any, Dict, List, NamedTuple, Optional, Union, overload +from typing import TYPE_CHECKING, Any, Dict, Hashable, List, NamedTuple, Optional, Union, overload +from typing_extensions import Protocol from unittest.mock import patch import warnings @@ -42,7 +43,16 @@ compresslevel = 9 -def create_seqinfo(mw: dw.Wrapper, series_files: list[str], series_id: str) -> SeqInfo: +class CustomSeqinfoT(Protocol): + def __call__(self, wrapper: dw.Wrapper, series_files: list[str]) -> Hashable: ... + + +def create_seqinfo( + mw: dw.Wrapper, + series_files: list[str], + series_id: str, + custom_seqinfo: CustomSeqinfoT | None = None, +) -> SeqInfo: """Generate sequence info Parameters @@ -109,6 +119,9 @@ def create_seqinfo(mw: dw.Wrapper, series_files: list[str], series_id: str) -> S date=dcminfo.get("AcquisitionDate"), series_uid=dcminfo.get("SeriesInstanceUID"), time=dcminfo.get("AcquisitionTime"), + custom = + custom_seqinfo(wrapper=mw, series_files=series_files) + if custom_seqinfo else None, ) @@ -199,6 +212,7 @@ def group_dicoms_into_seqinfos( dict[SeqInfo, list[str]], ] | None = None, + custom_seqinfo: CustomSeqinfoT | None = None, ) -> dict[SeqInfo, list[str]]: ... @@ -215,6 +229,7 @@ def group_dicoms_into_seqinfos( dict[SeqInfo, list[str]], ] | None = None, + custom_seqinfo: CustomSeqinfoT | None = None, ) -> dict[Optional[str], dict[SeqInfo, list[str]]] | dict[SeqInfo, list[str]]: """Process list of dicoms and return seqinfo and file group `seqinfo` contains per-sequence extract of fields from DICOMs which @@ -236,9 +251,11 @@ def group_dicoms_into_seqinfos( Creates a flattened `seqinfo` with corresponding DICOM files. True when invoked with `dicom_dir_template`. custom_grouping: str or callable, optional - grouping key defined within heuristic. Can be a string of a - DICOM attribute, or a method that handles more complex groupings. - + grouping key defined within heuristic. Can be a string of a + DICOM attribute, or a method that handles more complex groupings. + custom_seqinfo: callable, optional + A callable which will be provided MosaicWrapper giving possibility to + extract any custom DICOM metadata of interest. Returns ------- @@ -358,7 +375,7 @@ def group_dicoms_into_seqinfos( else: # nothing to see here, just move on continue - seqinfo = create_seqinfo(mw, series_files, series_id_str) + seqinfo = create_seqinfo(mw, series_files, series_id_str, custom_seqinfo) key: Optional[str] if per_studyUID: diff --git a/heudiconv/utils.py b/heudiconv/utils.py index 9f988267..f7bf16a7 100644 --- a/heudiconv/utils.py +++ b/heudiconv/utils.py @@ -24,6 +24,7 @@ from typing import ( Any, AnyStr, + Hashable, Mapping, NamedTuple, Optional, @@ -69,6 +70,7 @@ class SeqInfo(NamedTuple): date: Optional[str] # 24 series_uid: Optional[str] # 25 time: Optional[str] # 26 + custom: Optional[Hashable] # 27 class StudySessionInfo(NamedTuple): From a699e4d75d7cad82d9736e06f80181bcd9e1cefc Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Tue, 19 Jul 2022 16:10:25 -0400 Subject: [PATCH 02/18] TMP: just a demonstration on how custom_seqinfo could/should be used --- heudiconv/heuristics/convertall.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/heudiconv/heuristics/convertall.py b/heudiconv/heuristics/convertall.py index 8e1edee6..1ce783fa 100644 --- a/heudiconv/heuristics/convertall.py +++ b/heudiconv/heuristics/convertall.py @@ -1,7 +1,8 @@ from __future__ import annotations -from typing import Optional +from typing import Any, Optional +from heudiconv.dicoms import dw from heudiconv.utils import SeqInfo @@ -15,6 +16,14 @@ def create_key( return (template, outtype, annotation_classes) +def custom_seqinfo(wrapper: dw.Wrapper, series_files: list[str], **kw: Any) -> tuple[str, str]: + # Just a dummy demo for what custom_seqinfo could get/do + # for already loaded DICOM data, and including storing/returning + # the sample series file as was requested + # in https://github.com/nipy/heudiconv/pull/333 + return wrapper.affine, series_files[0] + + def infotodict( seqinfo: list[SeqInfo], ) -> dict[tuple[str, tuple[str, ...], None], list[str]]: From 48f14bfd1ffb222be8f58d02a2fef0fe8682c430 Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Fri, 28 Apr 2023 21:24:55 -0400 Subject: [PATCH 03/18] fixup typing in dicoms.py --- heudiconv/dicoms.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/heudiconv/dicoms.py b/heudiconv/dicoms.py index 7cdade8c..ba21c214 100644 --- a/heudiconv/dicoms.py +++ b/heudiconv/dicoms.py @@ -194,6 +194,7 @@ def group_dicoms_into_seqinfos( dict[SeqInfo, list[str]], ] | None = None, + custom_seqinfo: CustomSeqinfoT | None = None, ) -> dict[Optional[str], dict[SeqInfo, list[str]]]: ... @@ -212,7 +213,7 @@ def group_dicoms_into_seqinfos( dict[SeqInfo, list[str]], ] | None = None, - custom_seqinfo: CustomSeqinfoT | None = None, + custom_seqinfo: CustomSeqinfoT | None = None, ) -> dict[SeqInfo, list[str]]: ... From 4afc7dfec8169da98c833a7fb08888429bde475e Mon Sep 17 00:00:00 2001 From: basile Date: Wed, 15 Feb 2023 12:00:32 -0500 Subject: [PATCH 04/18] add custom_info to group_dicoms_into_seqinfos call in parser --- heudiconv/parser.py | 1 + 1 file changed, 1 insertion(+) diff --git a/heudiconv/parser.py b/heudiconv/parser.py index 27d822e1..d2d75084 100644 --- a/heudiconv/parser.py +++ b/heudiconv/parser.py @@ -224,6 +224,7 @@ def get_study_sessions( file_filter=getattr(heuristic, "filter_files", None), dcmfilter=getattr(heuristic, "filter_dicom", None), custom_grouping=getattr(heuristic, "grouping", None), + custom_seqinfo=getattr(heuristic, 'custom_seqinfo', None), ) if sids: From 115b2274b47356578020b8f0997cc38080e93572 Mon Sep 17 00:00:00 2001 From: basile Date: Thu, 16 Feb 2023 10:30:49 -0500 Subject: [PATCH 05/18] fix test, custom_seqinfo has to be hashable --- heudiconv/heuristics/convertall.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/heudiconv/heuristics/convertall.py b/heudiconv/heuristics/convertall.py index 1ce783fa..2bab26b6 100644 --- a/heudiconv/heuristics/convertall.py +++ b/heudiconv/heuristics/convertall.py @@ -21,7 +21,7 @@ def custom_seqinfo(wrapper: dw.Wrapper, series_files: list[str], **kw: Any) -> t # for already loaded DICOM data, and including storing/returning # the sample series file as was requested # in https://github.com/nipy/heudiconv/pull/333 - return wrapper.affine, series_files[0] + return wrapper.affine.tostring(), series_files[0] def infotodict( From a37e276c7546ef61f309e4aaf4bc69acede671e9 Mon Sep 17 00:00:00 2001 From: basile Date: Thu, 16 Feb 2023 10:45:16 -0500 Subject: [PATCH 06/18] test data include messed-up dicoms with no affine --- heudiconv/heuristics/convertall.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/heudiconv/heuristics/convertall.py b/heudiconv/heuristics/convertall.py index 2bab26b6..20f52019 100644 --- a/heudiconv/heuristics/convertall.py +++ b/heudiconv/heuristics/convertall.py @@ -21,7 +21,12 @@ def custom_seqinfo(wrapper: dw.Wrapper, series_files: list[str], **kw: Any) -> t # for already loaded DICOM data, and including storing/returning # the sample series file as was requested # in https://github.com/nipy/heudiconv/pull/333 - return wrapper.affine.tostring(), series_files[0] + from nibabel.nicom.dicomwrappers import WrapperError + try: + affine = wrapper.affine.tostring() + except WrapperError: + affine = None + return affine, series_files[0] def infotodict( From 803bbafd87a6c2e562a8826ba1927bfd8f0b406d Mon Sep 17 00:00:00 2001 From: basile Date: Thu, 16 Feb 2023 15:21:59 -0500 Subject: [PATCH 07/18] add a check for hashable custom_info, link to new doc section --- docs/heuristics.rst | 13 +++++++++++++ heudiconv/dicoms.py | 13 ++++++++++--- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/docs/heuristics.rst b/docs/heuristics.rst index 8c9a68d1..f7940d43 100644 --- a/docs/heuristics.rst +++ b/docs/heuristics.rst @@ -119,6 +119,19 @@ or:: ... return seqinfos # ordered dict containing seqinfo objects: list of DICOMs +--------------------------------------------------------------- +``custom_seqinfo(series_files, wrapper)`` +--------------------------------------------------------------- +If present this function will be called on eacg group of dicoms with +a sample nibabel dicom wrapper to extract additional information +to be used in ``infotodict``. + +Importantly the return value of that function needs to be hashable. +For instance the following non-hashable types can be converted to an alternative +hashable type: +- list > tuple +- dict > frozendict +- arrays > bytes (tobytes(), or pickle.dumps), str or tuple of tuples. ------------------------------- ``POPULATE_INTENDED_FOR_OPTS`` diff --git a/heudiconv/dicoms.py b/heudiconv/dicoms.py index ba21c214..67da0a75 100644 --- a/heudiconv/dicoms.py +++ b/heudiconv/dicoms.py @@ -90,6 +90,15 @@ def create_seqinfo( global total_files total_files += len(series_files) + custom_seqinfo_data = custom_seqinfo(wrapper=mw, series_files=series_files) \ + if custom_seqinfo else None + try: + hash(custom_seqinfo_data) + except TypeError: + raise RuntimeError("Data returned by the heuristics custom_seqinfo is not hashable. " + "See https://heudiconv.readthedocs.io/en/latest/heuristics.html#custom_seqinfo for more " + "details.") + return SeqInfo( total_files_till_now=total_files, example_dcm_file=op.basename(series_files[0]), @@ -119,9 +128,7 @@ def create_seqinfo( date=dcminfo.get("AcquisitionDate"), series_uid=dcminfo.get("SeriesInstanceUID"), time=dcminfo.get("AcquisitionTime"), - custom = - custom_seqinfo(wrapper=mw, series_files=series_files) - if custom_seqinfo else None, + custom=custom_seqinfo_data, ) From 678f8168b84bf45e60d5d33561cb546a7da123a4 Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Fri, 3 Mar 2023 12:43:38 -0500 Subject: [PATCH 08/18] Typo fix --- docs/heuristics.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/heuristics.rst b/docs/heuristics.rst index f7940d43..3f32a55a 100644 --- a/docs/heuristics.rst +++ b/docs/heuristics.rst @@ -122,7 +122,7 @@ or:: --------------------------------------------------------------- ``custom_seqinfo(series_files, wrapper)`` --------------------------------------------------------------- -If present this function will be called on eacg group of dicoms with +If present this function will be called on each group of dicoms with a sample nibabel dicom wrapper to extract additional information to be used in ``infotodict``. From 0f5846a2753528addf80ff1b9c4d3a82a08217d7 Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Fri, 3 Mar 2023 12:48:48 -0500 Subject: [PATCH 09/18] Log exception (as error) if we fail to obtain affine in convertall --- heudiconv/heuristics/convertall.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/heudiconv/heuristics/convertall.py b/heudiconv/heuristics/convertall.py index 20f52019..9afede7d 100644 --- a/heudiconv/heuristics/convertall.py +++ b/heudiconv/heuristics/convertall.py @@ -1,10 +1,13 @@ from __future__ import annotations +import logging from typing import Any, Optional from heudiconv.dicoms import dw from heudiconv.utils import SeqInfo +lgr = logging.getLogger('heudiconv') + def create_key( template: Optional[str], @@ -25,6 +28,7 @@ def custom_seqinfo(wrapper: dw.Wrapper, series_files: list[str], **kw: Any) -> t try: affine = wrapper.affine.tostring() except WrapperError: + lgr.exception("Errored out while obtaining/converting affine") affine = None return affine, series_files[0] From c640ffe0fa827f1b50323d5c773ca262ac1b3fb4 Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Thu, 20 Jul 2023 11:26:30 -0400 Subject: [PATCH 10/18] fix the order of args in the custom_seqinfo doc --- docs/heuristics.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/heuristics.rst b/docs/heuristics.rst index 3f32a55a..b55f6040 100644 --- a/docs/heuristics.rst +++ b/docs/heuristics.rst @@ -120,7 +120,7 @@ or:: return seqinfos # ordered dict containing seqinfo objects: list of DICOMs --------------------------------------------------------------- -``custom_seqinfo(series_files, wrapper)`` +``custom_seqinfo(wrapper, series_files)`` --------------------------------------------------------------- If present this function will be called on each group of dicoms with a sample nibabel dicom wrapper to extract additional information From 517ffdcec29eb982e5f0dc1f12be90c42d96ead2 Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Tue, 19 Jul 2022 16:10:08 -0400 Subject: [PATCH 11/18] ENH: custom_seqinfo - provide a way for heuristics to extract/add arbitrary value Just a draft implementation --- heudiconv/dicoms.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/heudiconv/dicoms.py b/heudiconv/dicoms.py index 67da0a75..d2c10f89 100644 --- a/heudiconv/dicoms.py +++ b/heudiconv/dicoms.py @@ -9,8 +9,7 @@ from pathlib import Path import sys import tarfile -from typing import TYPE_CHECKING, Any, Dict, Hashable, List, NamedTuple, Optional, Union, overload -from typing_extensions import Protocol +from typing import TYPE_CHECKING, Any, Dict, List, NamedTuple, Optional, Union, Protocol, cast, overload from unittest.mock import patch import warnings From 2afe136ba0033a28e9817e980229a79769a91399 Mon Sep 17 00:00:00 2001 From: bpinsard Date: Tue, 16 Jan 2024 09:16:11 -0500 Subject: [PATCH 12/18] change tostr -> tobytes for deprecation, add basic test --- heudiconv/heuristics/convertall.py | 2 +- heudiconv/tests/test_dicoms.py | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/heudiconv/heuristics/convertall.py b/heudiconv/heuristics/convertall.py index 9afede7d..d9ada9f7 100644 --- a/heudiconv/heuristics/convertall.py +++ b/heudiconv/heuristics/convertall.py @@ -26,7 +26,7 @@ def custom_seqinfo(wrapper: dw.Wrapper, series_files: list[str], **kw: Any) -> t # in https://github.com/nipy/heudiconv/pull/333 from nibabel.nicom.dicomwrappers import WrapperError try: - affine = wrapper.affine.tostring() + affine = wrapper.affine.tobytes() except WrapperError: lgr.exception("Errored out while obtaining/converting affine") affine = None diff --git a/heudiconv/tests/test_dicoms.py b/heudiconv/tests/test_dicoms.py index dc03790a..678ff2e2 100644 --- a/heudiconv/tests/test_dicoms.py +++ b/heudiconv/tests/test_dicoms.py @@ -99,6 +99,26 @@ def test_group_dicoms_into_seqinfos() -> None: ] +def test_custom_seqinfo() -> None: + """Tests for custom seqinfo extraction""" + + from heudiconv.heuristics.convertall import custom_seqinfo + + dcmfiles = glob(op.join(TESTS_DATA_PATH, "phantom.dcm")) + + seqinfos = group_dicoms_into_seqinfos( + dcmfiles, + "studyUID", + flatten=True, + custom_seqinfo=custom_seqinfo) + + seqinfo = list(seqinfos.keys())[0] + + assert hasattr(seqinfo, 'custom') + assert isinstance(seqinfo.custom, tuple) + assert len(seqinfo.custom) == 2 + assert seqinfo.custom[1] == dcmfiles[0] + def test_get_datetime_from_dcm_from_acq_date_time() -> None: typical_dcm = dcm.dcmread( op.join(TESTS_DATA_PATH, "phantom.dcm"), stop_before_pixels=True From b3193f678a4ef99348b7853821d85973d864e522 Mon Sep 17 00:00:00 2001 From: bpinsard Date: Tue, 16 Jan 2024 09:27:00 -0500 Subject: [PATCH 13/18] linting/typing --- heudiconv/convert.py | 2 +- heudiconv/dicoms.py | 3 ++- heudiconv/heuristics/convertall.py | 4 ++-- heudiconv/tests/test_dicoms.py | 1 + 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/heudiconv/convert.py b/heudiconv/convert.py index 40a575e7..05534dc9 100644 --- a/heudiconv/convert.py +++ b/heudiconv/convert.py @@ -223,7 +223,7 @@ def prep_conversion( custom_grouping=getattr(heuristic, "grouping", None), # callable which will be provided dcminfo and returned # structure extend seqinfo - custom_seqinfo = getattr(heuristic, 'custom_seqinfo', None), + custom_seqinfo=getattr(heuristic, 'custom_seqinfo', None), ) elif seqinfo is None: raise ValueError("Neither 'dicoms' nor 'seqinfo' is given") diff --git a/heudiconv/dicoms.py b/heudiconv/dicoms.py index d2c10f89..1dc11c7d 100644 --- a/heudiconv/dicoms.py +++ b/heudiconv/dicoms.py @@ -43,7 +43,8 @@ class CustomSeqinfoT(Protocol): - def __call__(self, wrapper: dw.Wrapper, series_files: list[str]) -> Hashable: ... + def __call__(self, wrapper: dw.Wrapper, series_files: list[str]) -> Hashable: + ... def create_seqinfo( diff --git a/heudiconv/heuristics/convertall.py b/heudiconv/heuristics/convertall.py index d9ada9f7..44cd5ede 100644 --- a/heudiconv/heuristics/convertall.py +++ b/heudiconv/heuristics/convertall.py @@ -1,7 +1,7 @@ from __future__ import annotations import logging -from typing import Any, Optional +from typing import Optional from heudiconv.dicoms import dw from heudiconv.utils import SeqInfo @@ -19,7 +19,7 @@ def create_key( return (template, outtype, annotation_classes) -def custom_seqinfo(wrapper: dw.Wrapper, series_files: list[str], **kw: Any) -> tuple[str, str]: +def custom_seqinfo(wrapper: dw.Wrapper, series_files: list[str]) -> tuple[str, str]: # Just a dummy demo for what custom_seqinfo could get/do # for already loaded DICOM data, and including storing/returning # the sample series file as was requested diff --git a/heudiconv/tests/test_dicoms.py b/heudiconv/tests/test_dicoms.py index 678ff2e2..8fab01be 100644 --- a/heudiconv/tests/test_dicoms.py +++ b/heudiconv/tests/test_dicoms.py @@ -119,6 +119,7 @@ def test_custom_seqinfo() -> None: assert len(seqinfo.custom) == 2 assert seqinfo.custom[1] == dcmfiles[0] + def test_get_datetime_from_dcm_from_acq_date_time() -> None: typical_dcm = dcm.dcmread( op.join(TESTS_DATA_PATH, "phantom.dcm"), stop_before_pixels=True From 419ed4824c915f06c7b0ff9d0f6805a39188e368 Mon Sep 17 00:00:00 2001 From: bpinsard Date: Tue, 16 Jan 2024 10:13:50 -0500 Subject: [PATCH 14/18] fix py3.11 typing import --- heudiconv/dicoms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/heudiconv/dicoms.py b/heudiconv/dicoms.py index 1dc11c7d..b7574c29 100644 --- a/heudiconv/dicoms.py +++ b/heudiconv/dicoms.py @@ -9,7 +9,7 @@ from pathlib import Path import sys import tarfile -from typing import TYPE_CHECKING, Any, Dict, List, NamedTuple, Optional, Union, Protocol, cast, overload +from typing import TYPE_CHECKING, Any, Dict, Hashable, List, NamedTuple, Optional, Union, Protocol, overload from unittest.mock import patch import warnings From cfdd3d70bf76e1c97565aabc5f29391efea9ccb1 Mon Sep 17 00:00:00 2001 From: bpinsard Date: Tue, 16 Jan 2024 16:08:11 -0500 Subject: [PATCH 15/18] run pre-commit magic! --- README.rst | 2 +- heudiconv/convert.py | 2 +- heudiconv/dicoms.py | 28 ++++++++++++++++++++++------ heudiconv/heuristics/convertall.py | 3 ++- heudiconv/parser.py | 2 +- heudiconv/tests/test_dicoms.py | 8 +++----- 6 files changed, 30 insertions(+), 15 deletions(-) diff --git a/README.rst b/README.rst index a177af8f..02a33c40 100644 --- a/README.rst +++ b/README.rst @@ -71,7 +71,7 @@ You can run your conversion automatically (which will produce a ``.heudiconv`` d .. image:: figs/workflow.png -``heudiconv`` comes with `existing heuristics `_ which can be used as is, or as examples. +``heudiconv`` comes with `existing heuristics `_ which can be used as is, or as examples. For instance, the Heuristic `convertall `_ extracts standard metadata from all matching DICOMs. ``heudiconv`` creates mapping files, ``.edit.text`` which lets researchers simply establish their own conversion mapping. diff --git a/heudiconv/convert.py b/heudiconv/convert.py index 05534dc9..aecc70dc 100644 --- a/heudiconv/convert.py +++ b/heudiconv/convert.py @@ -223,7 +223,7 @@ def prep_conversion( custom_grouping=getattr(heuristic, "grouping", None), # callable which will be provided dcminfo and returned # structure extend seqinfo - custom_seqinfo=getattr(heuristic, 'custom_seqinfo', None), + custom_seqinfo=getattr(heuristic, "custom_seqinfo", None), ) elif seqinfo is None: raise ValueError("Neither 'dicoms' nor 'seqinfo' is given") diff --git a/heudiconv/dicoms.py b/heudiconv/dicoms.py index b7574c29..f504af18 100644 --- a/heudiconv/dicoms.py +++ b/heudiconv/dicoms.py @@ -9,7 +9,18 @@ from pathlib import Path import sys import tarfile -from typing import TYPE_CHECKING, Any, Dict, Hashable, List, NamedTuple, Optional, Union, Protocol, overload +from typing import ( + TYPE_CHECKING, + Any, + Dict, + Hashable, + List, + NamedTuple, + Optional, + Protocol, + Union, + overload, +) from unittest.mock import patch import warnings @@ -90,14 +101,19 @@ def create_seqinfo( global total_files total_files += len(series_files) - custom_seqinfo_data = custom_seqinfo(wrapper=mw, series_files=series_files) \ - if custom_seqinfo else None + custom_seqinfo_data = ( + custom_seqinfo(wrapper=mw, series_files=series_files) + if custom_seqinfo + else None + ) try: hash(custom_seqinfo_data) except TypeError: - raise RuntimeError("Data returned by the heuristics custom_seqinfo is not hashable. " - "See https://heudiconv.readthedocs.io/en/latest/heuristics.html#custom_seqinfo for more " - "details.") + raise RuntimeError( + "Data returned by the heuristics custom_seqinfo is not hashable. " + "See https://heudiconv.readthedocs.io/en/latest/heuristics.html#custom_seqinfo for more " + "details." + ) return SeqInfo( total_files_till_now=total_files, diff --git a/heudiconv/heuristics/convertall.py b/heudiconv/heuristics/convertall.py index 44cd5ede..c232415a 100644 --- a/heudiconv/heuristics/convertall.py +++ b/heudiconv/heuristics/convertall.py @@ -6,7 +6,7 @@ from heudiconv.dicoms import dw from heudiconv.utils import SeqInfo -lgr = logging.getLogger('heudiconv') +lgr = logging.getLogger("heudiconv") def create_key( @@ -25,6 +25,7 @@ def custom_seqinfo(wrapper: dw.Wrapper, series_files: list[str]) -> tuple[str, s # the sample series file as was requested # in https://github.com/nipy/heudiconv/pull/333 from nibabel.nicom.dicomwrappers import WrapperError + try: affine = wrapper.affine.tobytes() except WrapperError: diff --git a/heudiconv/parser.py b/heudiconv/parser.py index d2d75084..cd891ac9 100644 --- a/heudiconv/parser.py +++ b/heudiconv/parser.py @@ -224,7 +224,7 @@ def get_study_sessions( file_filter=getattr(heuristic, "filter_files", None), dcmfilter=getattr(heuristic, "filter_dicom", None), custom_grouping=getattr(heuristic, "grouping", None), - custom_seqinfo=getattr(heuristic, 'custom_seqinfo', None), + custom_seqinfo=getattr(heuristic, "custom_seqinfo", None), ) if sids: diff --git a/heudiconv/tests/test_dicoms.py b/heudiconv/tests/test_dicoms.py index 8fab01be..b1ad0038 100644 --- a/heudiconv/tests/test_dicoms.py +++ b/heudiconv/tests/test_dicoms.py @@ -107,14 +107,12 @@ def test_custom_seqinfo() -> None: dcmfiles = glob(op.join(TESTS_DATA_PATH, "phantom.dcm")) seqinfos = group_dicoms_into_seqinfos( - dcmfiles, - "studyUID", - flatten=True, - custom_seqinfo=custom_seqinfo) + dcmfiles, "studyUID", flatten=True, custom_seqinfo=custom_seqinfo + ) seqinfo = list(seqinfos.keys())[0] - assert hasattr(seqinfo, 'custom') + assert hasattr(seqinfo, "custom") assert isinstance(seqinfo.custom, tuple) assert len(seqinfo.custom) == 2 assert seqinfo.custom[1] == dcmfiles[0] From 4688912a7c1121f3d889c32f8cdb1f926f5074a9 Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Tue, 13 Feb 2024 11:29:46 -0500 Subject: [PATCH 16/18] Create convertall_custom.py as a demonstration for a derived heuristic with custom_seqinfo --- heudiconv/heuristics/convertall.py | 16 --------------- heudiconv/heuristics/convertall_custom.py | 25 +++++++++++++++++++++++ 2 files changed, 25 insertions(+), 16 deletions(-) create mode 100644 heudiconv/heuristics/convertall_custom.py diff --git a/heudiconv/heuristics/convertall.py b/heudiconv/heuristics/convertall.py index c232415a..dc5ef073 100644 --- a/heudiconv/heuristics/convertall.py +++ b/heudiconv/heuristics/convertall.py @@ -3,7 +3,6 @@ import logging from typing import Optional -from heudiconv.dicoms import dw from heudiconv.utils import SeqInfo lgr = logging.getLogger("heudiconv") @@ -19,21 +18,6 @@ def create_key( return (template, outtype, annotation_classes) -def custom_seqinfo(wrapper: dw.Wrapper, series_files: list[str]) -> tuple[str, str]: - # Just a dummy demo for what custom_seqinfo could get/do - # for already loaded DICOM data, and including storing/returning - # the sample series file as was requested - # in https://github.com/nipy/heudiconv/pull/333 - from nibabel.nicom.dicomwrappers import WrapperError - - try: - affine = wrapper.affine.tobytes() - except WrapperError: - lgr.exception("Errored out while obtaining/converting affine") - affine = None - return affine, series_files[0] - - def infotodict( seqinfo: list[SeqInfo], ) -> dict[tuple[str, tuple[str, ...], None], list[str]]: diff --git a/heudiconv/heuristics/convertall_custom.py b/heudiconv/heuristics/convertall_custom.py new file mode 100644 index 00000000..abc4f812 --- /dev/null +++ b/heudiconv/heuristics/convertall_custom.py @@ -0,0 +1,25 @@ +"""A demo convertall heuristic with custom_seqinfo extracting affine and sample DICOM path + +This heuristic also demonstrates on how to create a "derived" heuristic which would augment +behavior of an already existing heuristic without complete rewrite. Such approach could be +useful for heuristic like reproin to overload mapping etc. +""" + +from .convertall import * # noqa: F403 + + +def custom_seqinfo(series_files, wrapper, **kw): # noqa: U100 + """Demo for extracting custom header fields into custom_seqinfo field + + Operates on already loaded DICOM data. + Origin: https://github.com/nipy/heudiconv/pull/333 + """ + + from nibabel.nicom.dicomwrappers import WrapperError + + try: + affine = wrapper.affine.tostring() + except WrapperError: + lgr.exception("Errored out while obtaining/converting affine") # noqa: F405 + affine = None + return affine, series_files[0] From ca48c66bffb894eaba08822e1bf46cdda5a8fece Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Tue, 13 Feb 2024 11:46:42 -0500 Subject: [PATCH 17/18] Fix import in the test due to move + use str() since now tostring is gone --- heudiconv/heuristics/convertall_custom.py | 2 +- heudiconv/tests/test_dicoms.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/heudiconv/heuristics/convertall_custom.py b/heudiconv/heuristics/convertall_custom.py index abc4f812..780ecc76 100644 --- a/heudiconv/heuristics/convertall_custom.py +++ b/heudiconv/heuristics/convertall_custom.py @@ -18,7 +18,7 @@ def custom_seqinfo(series_files, wrapper, **kw): # noqa: U100 from nibabel.nicom.dicomwrappers import WrapperError try: - affine = wrapper.affine.tostring() + affine = str(wrapper.affine) except WrapperError: lgr.exception("Errored out while obtaining/converting affine") # noqa: F405 affine = None diff --git a/heudiconv/tests/test_dicoms.py b/heudiconv/tests/test_dicoms.py index b1ad0038..12ac29c6 100644 --- a/heudiconv/tests/test_dicoms.py +++ b/heudiconv/tests/test_dicoms.py @@ -102,7 +102,7 @@ def test_group_dicoms_into_seqinfos() -> None: def test_custom_seqinfo() -> None: """Tests for custom seqinfo extraction""" - from heudiconv.heuristics.convertall import custom_seqinfo + from heudiconv.heuristics.convertall_custom import custom_seqinfo dcmfiles = glob(op.join(TESTS_DATA_PATH, "phantom.dcm")) From 41b22d7b8b8ff1a1e066515b17d5da4baebd6c56 Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Wed, 28 Feb 2024 09:59:07 -0500 Subject: [PATCH 18/18] Provide types for custom_seqinfo but disable type checking in the test -- I think mypy has deficiency since we do use kwargs and thus should be ok --- heudiconv/heuristics/convertall_custom.py | 9 ++++++++- heudiconv/tests/test_dicoms.py | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/heudiconv/heuristics/convertall_custom.py b/heudiconv/heuristics/convertall_custom.py index 780ecc76..26f0ca05 100644 --- a/heudiconv/heuristics/convertall_custom.py +++ b/heudiconv/heuristics/convertall_custom.py @@ -4,11 +4,18 @@ behavior of an already existing heuristic without complete rewrite. Such approach could be useful for heuristic like reproin to overload mapping etc. """ +from __future__ import annotations + +from typing import Any + +import nibabel.nicom.dicomwrappers as dw from .convertall import * # noqa: F403 -def custom_seqinfo(series_files, wrapper, **kw): # noqa: U100 +def custom_seqinfo( + series_files: list[str], wrapper: dw.Wrapper, **kw: Any # noqa: U100 +) -> tuple[str | None, str]: """Demo for extracting custom header fields into custom_seqinfo field Operates on already loaded DICOM data. diff --git a/heudiconv/tests/test_dicoms.py b/heudiconv/tests/test_dicoms.py index 12ac29c6..4cb28290 100644 --- a/heudiconv/tests/test_dicoms.py +++ b/heudiconv/tests/test_dicoms.py @@ -108,7 +108,7 @@ def test_custom_seqinfo() -> None: seqinfos = group_dicoms_into_seqinfos( dcmfiles, "studyUID", flatten=True, custom_seqinfo=custom_seqinfo - ) + ) # type: ignore seqinfo = list(seqinfos.keys())[0]