Merge pull request #1590 from AllenNeuralDynamics/1589-ensure-merging-files-valid-valid-outputs-a-valid-file

hotfix: move the subject_details validator out of Acquisition

Author: dbirman

src/aind_data_schema/__init__.py

@@ -1,4 +1,4 @@
 """ base module for aind-data-schema
 """
 
-__version__ = "2.0.4"
+__version__ = "2.0.5"

src/aind_data_schema/core/acquisition.py

@@ -311,14 +311,6 @@ class Acquisition(DataCoreModel):
     )
     subject_details: Optional[AcquisitionSubjectDetails] = Field(default=None, title="Subject details")
 
-    @model_validator(mode="after")
-    def subject_details_if_not_specimen(self):
-        """Check that subject details are present if no specimen ID"""
-        if not self.specimen_id and not self.subject_details:
-            raise ValueError("Subject details are required for in vivo experiments")
-
-        return self
-
     @model_validator(mode="after")
     def check_subject_specimen_id(self):
         """Check that the subject and specimen IDs match"""

src/aind_data_schema/core/metadata.py

@@ -133,6 +133,16 @@ def validate_core_fields(cls, value, info: ValidationInfo):
             core_model = value
         return core_model
 
+    @model_validator(mode="after")
+    def validate_subject_details_if_not_specimen(self):
+        """Check that subject details are present if an in vivo experiment"""
+
+        if self.acquisition and not self.acquisition.specimen_id:
+            if not self.acquisition.subject_details:
+                raise ValueError("Acquisition.subject_details are required for in vivo experiments")
+
+        return self
+
     @model_validator(mode="after")
     def validate_expected_files_by_modality(self):
         """Validator warns users if required files are missing"""

tests/test_acquisition.py

@@ -101,17 +101,6 @@ def test_constructors(self):
 
         assert mri is not None
 
-    def test_subject_details_if_not_specimen(self):
-        """Test that subject details are required if no specimen ID"""
-        with self.assertRaises(ValueError) as context:
-            acq = ephys_acquisition.model_copy()
-
-            acq.subject_details = None
-
-            Acquisition.model_validate_json(acq.model_dump_json())
-
-        self.assertIn("Subject details are required for in vivo experiments", str(context.exception))
-
     def test_check_subject_specimen_id(self):
         """Test that subject and specimen IDs match"""
         with self.assertRaises(ValueError) as context:

tests/test_metadata.py

@@ -884,6 +884,54 @@ def test_validate_calibration_object_tags(self):
         )
         self.assertIsNotNone(metadata)
 
+    def test_validate_subject_details_if_not_specimen(self):
+        """Tests that subject details are required if acquisition.specimen_id is not provided"""
+
+        # Case where specimen_id is provided - should pass without subject_details
+        acquisition_with_specimen = Acquisition.model_construct(
+            instrument_id="Test",
+            subject_id="123456",
+            specimen_id="123456-001",
+            data_streams=[],
+        )
+        metadata_with_specimen = Metadata(
+            name="Test Metadata",
+            location="Test Location",
+            subject=subject,
+            acquisition=acquisition_with_specimen,
+        )
+        self.assertIsNotNone(metadata_with_specimen)
+
+        # Case where specimen_id is not provided and subject_details is provided - should pass
+        acquisition_with_details = Acquisition.model_construct(
+            instrument_id="Test",
+            subject_id="123456",
+            data_streams=[],
+            subject_details=AcquisitionSubjectDetails.model_construct(),
+        )
+        metadata_with_details = Metadata(
+            name="Test Metadata",
+            location="Test Location",
+            subject=subject,
+            acquisition=acquisition_with_details,
+        )
+        self.assertIsNotNone(metadata_with_details)
+
+        # Case where neither specimen_id nor subject_details is provided - should fail
+        acquisition_missing_both = Acquisition.model_construct(
+            subject_id="123456",
+            instrument_id="Test",
+            data_streams=[],
+        )
+        with self.assertRaises(ValueError) as context:
+            Metadata(
+                name="Test Metadata",
+                location="Test Location",
+                subject=subject,
+                acquisition=acquisition_missing_both,
+            )
+        self.assertIn("Acquisition.subject_details are required for in vivo experiments", str(context.exception))
+
 
 if __name__ == "__main__":
     unittest.main()