From f580c8b24599152c8e955eae0cc0cd6bdb9477ac Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Fri, 20 Feb 2026 09:54:34 -0500 Subject: [PATCH] Fix time-of-day-dependent failure in series_time validation The series_time > content_time check was performed regardless of whether series_date and content_date differed. When content_time defaults to now.time(), this caused spurious failures depending on what time of day the code (or tests) ran. The time comparison is now only performed when the dates are equal. Co-Authored-By: Claude Opus 4.6 --- src/highdicom/base.py | 14 ++++++++----- tests/test_base.py | 49 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 57 insertions(+), 6 deletions(-) diff --git a/src/highdicom/base.py b/src/highdicom/base.py index 93f3680a5..5f610a738 100644 --- a/src/highdicom/base.py +++ b/src/highdicom/base.py @@ -293,11 +293,15 @@ def __init__( "'series_time' may not be specified without " "'series_date'." ) - if content_time is not None: - if series_time > content_time: - raise ValueError( - "'series_time' must not be later than content time." - ) + if ( + content_time is not None and + content_date is not None and + series_date == content_date and + series_time > content_time + ): + raise ValueError( + "'series_time' must not be later than content time." + ) self.SeriesTime = series_time if content_qualification is not None: diff --git a/tests/test_base.py b/tests/test_base.py index 3f400a928..1e2d5c36c 100644 --- a/tests/test_base.py +++ b/tests/test_base.py @@ -99,12 +99,59 @@ def test_series_datetime(self): manufacturer_model_name='foo-bar', software_versions='v1.0.0', transfer_syntax_uid=ExplicitVRLittleEndian, - series_date=datetime.date(2000, 12, 1), + content_date=datetime.date(2024, 6, 15), + content_time=datetime.time(14, 0, 0), + series_date=datetime.date(2024, 6, 15), series_time=datetime.time(12, 34, 56), ) assert hasattr(instance, 'SeriesDate') assert hasattr(instance, 'SeriesTime') + def test_series_datetime_earlier_date(self): + # series_time > content_time is fine when series_date < content_date + instance = SOPClass( + study_instance_uid=UID(), + series_instance_uid=UID(), + series_number=1, + sop_instance_uid=UID(), + sop_class_uid='1.2.840.10008.5.1.4.1.1.88.33', + instance_number=1, + modality='SR', + manufacturer='highdicom', + manufacturer_model_name='foo-bar', + software_versions='v1.0.0', + transfer_syntax_uid=ExplicitVRLittleEndian, + content_date=datetime.date(2024, 6, 15), + content_time=datetime.time(8, 0, 0), + series_date=datetime.date(2024, 6, 14), + series_time=datetime.time(23, 59, 59), + ) + assert hasattr(instance, 'SeriesDate') + assert hasattr(instance, 'SeriesTime') + + def test_series_time_after_content_same_date(self): + msg = ( + "'series_time' must not be later than content time." + ) + with pytest.raises(ValueError, match=msg): + SOPClass( + study_instance_uid=UID(), + series_instance_uid=UID(), + series_number=1, + sop_instance_uid=UID(), + sop_class_uid='1.2.840.10008.5.1.4.1.1.88.33', + instance_number=1, + modality='SR', + manufacturer='highdicom', + manufacturer_model_name='foo-bar', + software_versions='v1.0.0', + transfer_syntax_uid=ExplicitVRLittleEndian, + content_date=datetime.date(2024, 6, 15), + content_time=datetime.time(8, 0, 0), + series_date=datetime.date(2024, 6, 15), + series_time=datetime.time(12, 34, 56), + ) + def test_series_date_without_time(self): msg = ( "'series_time' may not be specified without "