Skip to content

Commit 8799377

Browse files
authored
refactor: soften time validation (#1505)
* refactor: remove time validation from subject and reduce data_description to a warning * fix: wording * tests: remove test for validator that was removed * tests: additional coverage, and removing unnecessary validators * fix: revert changes to subject validation
1 parent 961761a commit 8799377

File tree

4 files changed

+51
-21
lines changed

4 files changed

+51
-21
lines changed

src/aind_data_schema/core/metadata.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -351,13 +351,13 @@ def validate_data_description_name_time_consistency(self):
351351
if name_creation_time:
352352
try:
353353
validate_creation_time_after_midnight(name_creation_time, self.acquisition.acquisition_end_time)
354-
except ValueError as e:
355-
# Re-raise with more specific context for data_description.name
356-
raise ValueError(
354+
except ValueError:
355+
# Issue a warning instead of raising an error
356+
warnings.warn(
357357
f"Creation time from data_description.name ({name_creation_time}) "
358-
f"must be on or after midnight of the acquisition day "
359-
f"({self.acquisition.acquisition_end_time.date()})"
360-
) from e
358+
f"should be close to the acquisition end time "
359+
f"({self.acquisition.acquisition_end_time})"
360+
)
361361

362362
return self
363363

src/aind_data_schema/utils/validators.py

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -90,19 +90,19 @@ def recursive_time_validation_check(data, acquisition_start_time=None, acquisiti
9090
_time_validation_recurse_helper(data, acquisition_start_time, acquisition_end_time)
9191

9292

93+
def _convert_to_comparable(value, reference_datetime):
94+
"""Convert date to datetime using the timezone from reference, or return as-is if already datetime"""
95+
if isinstance(value, date) and not isinstance(value, datetime):
96+
# Convert date to datetime at midnight with same timezone as reference
97+
return datetime.combine(value, datetime.min.time()).replace(tzinfo=reference_datetime.tzinfo)
98+
return value
99+
100+
93101
def _validate_time_constraint(field_value, time_validation, start_time, end_time, field_name):
94102
"""Validate a single time field against the specified constraint."""
95103

96-
# Handle conversion between date and datetime objects for comparison
97-
def convert_to_comparable(value, reference_datetime):
98-
"""Convert date to datetime using the timezone from reference, or return as-is if already datetime"""
99-
if isinstance(value, date) and not isinstance(value, datetime):
100-
# Convert date to datetime at midnight with same timezone as reference
101-
return datetime.combine(value, datetime.min.time()).replace(tzinfo=reference_datetime.tzinfo)
102-
return value
103-
104104
# Convert field_value to be comparable with start_time and end_time
105-
comparable_field_value = convert_to_comparable(field_value, start_time)
105+
comparable_field_value = _convert_to_comparable(field_value, start_time)
106106

107107
if time_validation == TimeValidation.BETWEEN:
108108
if not (start_time <= comparable_field_value <= end_time):

tests/test_metadata.py

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import json
44
import unittest
5+
import warnings
56
from datetime import datetime, timezone
67
from pathlib import Path
78

@@ -612,16 +613,21 @@ def test_validate_data_description_name_time_consistency(self):
612613
project_name="Test Project",
613614
)
614615

615-
# This should fail - creation time is before the acquisition day
616-
with self.assertRaises(ValidationError) as context:
617-
Metadata(
616+
# This should issue a warning - creation time is before the acquisition day
617+
with warnings.catch_warnings(record=True) as warning_list:
618+
warnings.simplefilter("always")
619+
metadata_with_warning = Metadata(
618620
name="Test Metadata",
619621
location="Test Location",
620622
data_description=data_description_before,
621623
acquisition=acquisition,
622624
)
623-
self.assertIn("Creation time from data_description.name", str(context.exception))
624-
self.assertIn("must be on or after midnight of the acquisition day", str(context.exception))
625+
# Should have successfully created the metadata object
626+
self.assertIsNotNone(metadata_with_warning)
627+
# Should have issued a warning
628+
self.assertEqual(len(warning_list), 1)
629+
self.assertIn("Creation time from data_description.name", str(warning_list[0].message))
630+
self.assertIn("should be close to the acquisition end time", str(warning_list[0].message))
625631

626632
# Test case where data_description is None (should pass)
627633
metadata_no_data_desc = Metadata(

tests/test_utils_validators.py

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
""" Tests for compatibility check utilities """
22

33
import unittest
4-
from datetime import datetime, timezone, timedelta
4+
from datetime import date, datetime, timezone, timedelta
55
from enum import Enum
66
from pathlib import Path
77
from typing import Annotated
@@ -17,6 +17,7 @@
1717
CoordinateSystemException,
1818
SystemNameException,
1919
TimeValidation,
20+
_convert_to_comparable,
2021
_recurse_helper,
2122
_system_check_helper,
2223
_time_validation_recurse_helper,
@@ -683,5 +684,28 @@ def test_invalid_date_object_creation_time(self):
683684
self.assertIn("must be on or after midnight", str(context.exception))
684685

685686

687+
class TestConvertToComparable(unittest.TestCase):
688+
"""Tests for _convert_to_comparable function"""
689+
690+
def test_convert_date_to_datetime(self):
691+
"""Test converting date to datetime with timezone from reference"""
692+
reference_time = datetime(2023, 1, 1, 12, 0, 0, tzinfo=timezone.utc)
693+
test_date = date(2023, 1, 2)
694+
695+
result = _convert_to_comparable(test_date, reference_time)
696+
697+
expected = datetime(2023, 1, 2, 0, 0, 0, tzinfo=timezone.utc)
698+
self.assertEqual(result, expected)
699+
700+
def test_return_datetime_unchanged(self):
701+
"""Test that datetime objects are returned unchanged"""
702+
reference_time = datetime(2023, 1, 1, 12, 0, 0, tzinfo=timezone.utc)
703+
test_datetime = datetime(2023, 1, 2, 10, 30, 0, tzinfo=timezone.utc)
704+
705+
result = _convert_to_comparable(test_datetime, reference_time)
706+
707+
self.assertEqual(result, test_datetime)
708+
709+
686710
if __name__ == "__main__":
687711
unittest.main()

0 commit comments

Comments
 (0)