diff --git a/adit/core/tests/utils/test_dicom_utils.py b/adit/core/tests/utils/test_dicom_utils.py index 2fe924c2..1052aa0d 100644 --- a/adit/core/tests/utils/test_dicom_utils.py +++ b/adit/core/tests/utils/test_dicom_utils.py @@ -61,10 +61,20 @@ def test_convert_to_python_date(): assert convert_to_python_date("20230831") == date(2023, 8, 31) +def test_convert_to_python_date_rejects_empty_value(): + with pytest.raises(ValueError): + convert_to_python_date("") + + def test_convert_to_python_time(): assert convert_to_python_time("080000") == time(8, 0, 0) +def test_convert_to_python_time_rejects_empty_value(): + with pytest.raises(ValueError): + convert_to_python_time("") + + def test_convert_to_python_regex_exact_match(): # No wildcard = exact whole-value match (DICOM Single Value Matching) pattern = convert_to_python_regex("query") diff --git a/adit/core/utils/dicom_utils.py b/adit/core/utils/dicom_utils.py index 9f60e47d..5c5703d8 100644 --- a/adit/core/utils/dicom_utils.py +++ b/adit/core/utils/dicom_utils.py @@ -143,16 +143,24 @@ def convert_to_dicom_datetime(value: str | datetime.datetime | DateTimeRange) -> def convert_to_python_date(value: str) -> datetime.date: - """Convert a DICOM date string to a Python date object.""" + """Convert a DICOM date string to a Python date object. + + Raises ValueError if the value is empty or invalid. + """ date_valuerep = valuerep.DA(value) - assert date_valuerep is not None + if date_valuerep is None: + raise ValueError(f"Empty DICOM date: {value!r}") return datetime.date.fromisoformat(date_valuerep.isoformat()) def convert_to_python_time(value: str) -> datetime.time: - """Convert a DICOM date string to a Python date object.""" + """Convert a DICOM time string to a Python time object. + + Raises ValueError if the value is empty or invalid. + """ time_valuerep = valuerep.TM(value) - assert time_valuerep is not None + if time_valuerep is None: + raise ValueError(f"Empty DICOM time: {value!r}") return datetime.time.fromisoformat(time_valuerep.isoformat()) diff --git a/adit/mass_transfer/tests/test_processor.py b/adit/mass_transfer/tests/test_processor.py index b662f98b..0631e38b 100644 --- a/adit/mass_transfer/tests/test_processor.py +++ b/adit/mass_transfer/tests/test_processor.py @@ -699,8 +699,17 @@ def test_discover_series_exclude_by_institution(mocker: MockerFixture): assert series_uids == {"1.2.3.901"} -def test_discover_series_salvages_malformed_study_time(mocker: MockerFixture): - """A non-conformant StudyTime (as returned by some PACS) falls back to midnight.""" +@pytest.mark.parametrize( + "study_time", + [ + # TM only allows a fractional part after full HHMMSS, so HHMM.fff is invalid + pytest.param("1113.672", id="non-conformant"), + # StudyTime is Type 2, so a PACS may return it empty + pytest.param("", id="empty"), + ], +) +def test_discover_series_salvages_unparseable_study_time(mocker: MockerFixture, study_time: str): + """A StudyTime that can't be parsed (as returned by some PACS) falls back to midnight.""" processor = _make_processor(mocker) processor.mass_task.partition_start = datetime(2024, 1, 1, 0, 0) processor.mass_task.partition_end = datetime(2024, 1, 1, 23, 59, 59) @@ -709,8 +718,7 @@ def test_discover_series_salvages_malformed_study_time(mocker: MockerFixture): operator.server = mocker.MagicMock(max_search_results=200) study = _make_study("1.2.3.100") - # TM only allows a fractional part after full HHMMSS, so HHMM.fff is invalid - _set_nonconformant(study.dataset, "StudyTime", "1113.672") + _set_nonconformant(study.dataset, "StudyTime", study_time) operator.find_studies.return_value = [study] operator.find_series.return_value = [_make_series_result("1.2.3.201")]