Skip to content

Commit

Permalink
RF: Gather/use list of all dicom files to avoid calling wrapper_from_…
Browse files Browse the repository at this point in the history
…file at module level

I think it is safer to avoid 3rd party library calls at the module import time
since if they are buggy it would be too loud of kaboom. Also it adds CPU time
at import time even if not used later on. So replaced with defining a list
of all DICOMs and then just using that function directly from filename.

This also would run it against more DICOMs as this time we would glob recursively.
  • Loading branch information
yarikoptic committed Sep 16, 2024
1 parent 9afeae9 commit 2862c9a
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 14 deletions.
20 changes: 6 additions & 14 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,21 +180,13 @@ 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,
) -> 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:
Expand Down
9 changes: 9 additions & 0 deletions heudiconv/tests/utils.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

from glob import glob
import logging
import os.path as op
from pathlib import Path
Expand All @@ -9,6 +10,14 @@

HEURISTICS_PATH = op.join(heudiconv.heuristics.__path__[0])
TESTS_DATA_PATH = op.join(op.dirname(__file__), "data")
# Do relative to curdir to shorten in a typical application,
# and side-effect test that tests do not change curdir.
TEST_DICOM_PATHS = [
op.relpath(x)
for x in glob(op.join(TESTS_DATA_PATH, "**/*.dcm"), recursive=True)
# exclude PhoenixDocuments
if "PhoenixDocument" not in x
]

lgr = logging.getLogger(__name__)

Expand Down

0 comments on commit 2862c9a

Please sign in to comment.