Skip to content

Commit 4915721

Browse files
Merge pull request #413 from AllenNeuralDynamics/fix_411-datetime-serialization
Fix json dump datetime bug and add test
2 parents 38d5198 + 6f27939 commit 4915721

2 files changed

Lines changed: 38 additions & 6 deletions

File tree

src/aind_metadata_mapper/gather_metadata.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ def _merge_models(self, model_class, models: list[dict]) -> dict:
356356
for model in model_objs[1:]:
357357
merged_model = merged_model + model
358358

359-
return merged_model.model_dump()
359+
return merged_model.model_dump(mode="json")
360360

361361
def get_instrument(self) -> Optional[dict]:
362362
"""Get instrument metadata"""

tests/test_gather_metadata.py

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,19 @@
11
"""Tests gather_metadata module"""
22

3+
import json
34
import os
45
import unittest
5-
from datetime import datetime
6+
from datetime import datetime, timezone
67
from pathlib import Path
78
from unittest.mock import MagicMock, mock_open, patch
89

910
from aind_data_schema.components.identifiers import Person
11+
from aind_data_schema.core.acquisition import Acquisition
1012
from aind_data_schema.core.metadata import Metadata
1113
from aind_data_schema_models.modalities import Modality
1214
from aind_data_schema_models.organizations import Organization
1315

14-
from aind_metadata_mapper.gather_metadata import (
15-
GatherMetadataJob,
16-
_metadata_service_helper,
17-
)
16+
from aind_metadata_mapper.gather_metadata import GatherMetadataJob, _metadata_service_helper
1817
from aind_metadata_mapper.models import JobSettings
1918

2019
TEST_DIR = Path(os.path.dirname(os.path.realpath(__file__)))
@@ -809,6 +808,39 @@ def test_merge_models_acquisition(self):
809808
self.assertIsInstance(result, dict)
810809
self.assertIn("acquisition_start_time", result)
811810

811+
def test_merge_models_datetime_serialization(self):
812+
"""Test that merged models can be JSON serialized (datetime objects converted to strings)"""
813+
# Create two minimal acquisition models with datetime fields
814+
now = datetime.now(timezone.utc)
815+
816+
acq1_dict = {
817+
"subject_id": "test",
818+
"acquisition_start_time": now.isoformat(),
819+
"acquisition_end_time": now.isoformat(),
820+
"instrument_id": "test",
821+
"acquisition_type": "test",
822+
"data_streams": [],
823+
}
824+
825+
acq2_dict = {
826+
"subject_id": "test",
827+
"acquisition_start_time": now.isoformat(),
828+
"acquisition_end_time": now.isoformat(),
829+
"instrument_id": "test",
830+
"acquisition_type": "test",
831+
"data_streams": [],
832+
}
833+
834+
# Merge the two acquisitions using _merge_models
835+
merged_dict = self.job._merge_models(Acquisition, [acq1_dict, acq2_dict])
836+
837+
# Verify the merged dict can be JSON serialized
838+
# This will fail if merged_dict contains datetime objects instead of strings
839+
json_str = json.dumps(merged_dict, indent=2)
840+
self.assertIsInstance(json_str, str)
841+
# Verify datetime fields are strings, not datetime objects
842+
self.assertIsInstance(merged_dict.get("acquisition_start_time"), str)
843+
812844
def test_get_instrument_multiple_files(self):
813845
"""Test get_instrument with multiple instrument files"""
814846
import json

0 commit comments

Comments
 (0)