Skip to content

Commit 79d3807

Browse files
authored
fix: [hotfix] add missing validator for one of required file sets (#1521)
* fix: [hotfix] add missing validator for one of required file sets * tests: fix tests to handle new validator * fix: properly construct Metadata when core files are missing, bypassing core file validators * tests: need coverage on condition with missing required files * chore: lint
1 parent baff6ca commit 79d3807

File tree

2 files changed

+53
-8
lines changed

2 files changed

+53
-8
lines changed

src/aind_data_schema/core/metadata.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,17 @@ def validate_expected_files_by_modality(self):
145145

146146
return self
147147

148+
@model_validator(mode="after")
149+
def validate_required_files(self):
150+
"""Validator to ensure that one of the key files from the file sets is present."""
151+
152+
one_of_required = REQUIRED_FILE_SETS.keys()
153+
154+
if not any(getattr(self, file) for file in one_of_required):
155+
raise ValueError(f"Metadata must contain at least one of the following files: {', '.join(one_of_required)}")
156+
157+
return self
158+
148159
@model_validator(mode="after")
149160
def validate_smartspim_metadata(self):
150161
"""Validator for smartspim metadata"""
@@ -394,7 +405,7 @@ def create_metadata_json(
394405
metadata_json = json.loads(metadata.model_dump_json(by_alias=True))
395406
except Exception as e:
396407
logging.warning(f"Issue with metadata construction! {e.args}")
397-
metadata = Metadata.model_validate(params)
408+
metadata = Metadata.model_construct(**params)
398409
metadata_json = json.loads(metadata.model_dump_json(by_alias=True))
399410
for key, value in core_fields.items():
400411
metadata_json[key] = value

tests/test_metadata.py

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
from aind_data_schema.core.acquisition import StimulusEpoch
3232

3333
from examples.data_description import d as data_description
34+
from examples.subject import s as subject
3435

3536

3637
EXAMPLES_DIR = Path(__file__).parents[1] / "examples"
@@ -146,7 +147,7 @@ def test_injection_material_validator_spim(self):
146147
subject_id="655019",
147148
data_level="raw",
148149
),
149-
subject=Subject.model_construct(),
150+
subject=subject,
150151
procedures=Procedures.model_construct(subject_procedures=[surgery2]),
151152
acquisition=Acquisition.model_construct(subject_details=AcquisitionSubjectDetails.model_construct()),
152153
instrument=self.spim_instrument,
@@ -171,7 +172,7 @@ def test_injection_material_validator_ephys(self):
171172
subject_id="655019",
172173
data_level="raw",
173174
),
174-
subject=Subject.model_construct(),
175+
subject=subject,
175176
procedures=Procedures.model_construct(subject_procedures=[surgery2]),
176177
instrument=ephys_inst,
177178
processing=Processing.model_construct(),
@@ -201,7 +202,7 @@ def test_validate_instrument_acquisition_compatibility(self):
201202
subject_id="655019",
202203
data_level="raw",
203204
),
204-
subject=Subject.model_construct(),
205+
subject=subject,
205206
procedures=Procedures.model_construct(),
206207
instrument=inst,
207208
processing=Processing.model_construct(),
@@ -221,6 +222,7 @@ def test_validate_old_schema_version(self):
221222
name="name",
222223
location="location",
223224
id="1",
225+
subject=subject,
224226
)
225227

226228
m_dict = m.model_dump()
@@ -273,7 +275,7 @@ def test_create_from_core_jsons_invalid(self):
273275
"data_description": None,
274276
"procedures": self.procedures_json,
275277
"instrument": Instrument.model_construct().model_dump(),
276-
"processing": Procedures.model_construct(injection_materials=["some materials"]).model_dump(),
278+
"processing": Processing.model_construct().model_dump(),
277279
"acquisition": None,
278280
"quality_control": None,
279281
}
@@ -319,6 +321,18 @@ def test_validate_expected_files_by_modality(self):
319321
self.assertIn("Metadata missing required file: instrument", warning_messages)
320322
self.assertIn("Metadata missing required file: acquisition", warning_messages)
321323

324+
# Test case where ALL required file set keys are missing (subject, processing, model)
325+
with self.assertRaises(ValueError) as context:
326+
Metadata(
327+
name="655019_2023-04-03T181709",
328+
location="bucket",
329+
# No subject, processing, or model - should trigger validation error
330+
)
331+
self.assertIn(
332+
"Metadata must contain at least one of the following files: subject, processing, model",
333+
str(context.exception),
334+
)
335+
322336
def test_validate_acquisition_connections(self):
323337
"""Tests that acquisition connections are validated correctly."""
324338
# Case where all connection devices are present in instrument components
@@ -340,6 +354,7 @@ def test_validate_acquisition_connections(self):
340354
metadata = Metadata(
341355
name="Test Metadata",
342356
location="Test Location",
357+
subject=subject,
343358
instrument=instrument,
344359
acquisition=acquisition,
345360
)
@@ -362,6 +377,7 @@ def test_validate_acquisition_connections(self):
362377
Metadata(
363378
name="Test Metadata",
364379
location="Test Location",
380+
subject=subject,
365381
instrument=instrument,
366382
acquisition=acquisition,
367383
)
@@ -387,6 +403,7 @@ def test_validate_acquisition_connections(self):
387403
Metadata(
388404
name="Test Metadata",
389405
location="Test Location",
406+
subject=subject,
390407
instrument=instrument,
391408
acquisition=acquisition_missing_source,
392409
)
@@ -416,6 +433,7 @@ def test_validate_acquisition_active_devices(self):
416433
metadata = Metadata(
417434
name="Test Metadata",
418435
location="Test Location",
436+
subject=subject,
419437
instrument=instrument,
420438
acquisition=acquisition,
421439
)
@@ -435,6 +453,7 @@ def test_validate_acquisition_active_devices(self):
435453
Metadata(
436454
name="Test Metadata",
437455
location="Test Location",
456+
subject=subject,
438457
instrument=instrument,
439458
acquisition=acquisition,
440459
)
@@ -461,6 +480,7 @@ def test_validate_training_protocol_references(self):
461480
metadata = Metadata(
462481
name="Test Metadata",
463482
location="Test Location",
483+
subject=subject,
464484
procedures=procedures,
465485
acquisition=acquisition,
466486
)
@@ -479,6 +499,7 @@ def test_validate_training_protocol_references(self):
479499
metadata = Metadata(
480500
name="Test Metadata",
481501
location="Test Location",
502+
subject=subject,
482503
procedures=procedures,
483504
acquisition=acquisition_invalid,
484505
)
@@ -499,6 +520,7 @@ def test_validate_training_protocol_references(self):
499520
metadata = Metadata(
500521
name="Test Metadata",
501522
location="Test Location",
523+
subject=subject,
502524
procedures=procedures_empty,
503525
acquisition=acquisition_invalid,
504526
)
@@ -525,6 +547,7 @@ def test_validate_training_protocol_references(self):
525547
metadata_none = Metadata(
526548
name="Test Metadata",
527549
location="Test Location",
550+
subject=subject,
528551
procedures=procedures,
529552
acquisition=acquisition_none,
530553
)
@@ -534,6 +557,7 @@ def test_validate_training_protocol_references(self):
534557
metadata_no_acquisition = Metadata(
535558
name="Test Metadata",
536559
location="Test Location",
560+
subject=subject,
537561
procedures=procedures,
538562
)
539563
self.assertIsNotNone(metadata_no_acquisition)
@@ -542,6 +566,7 @@ def test_validate_training_protocol_references(self):
542566
metadata_no_procedures = Metadata(
543567
name="Test Metadata",
544568
location="Test Location",
569+
subject=subject,
545570
acquisition=acquisition,
546571
)
547572
self.assertIsNotNone(metadata_no_procedures)
@@ -578,6 +603,7 @@ def test_validate_data_description_name_time_consistency(self):
578603
metadata_matching = Metadata(
579604
name="Test Metadata",
580605
location="Test Location",
606+
subject=subject,
581607
data_description=data_description,
582608
acquisition=acquisition,
583609
)
@@ -599,6 +625,7 @@ def test_validate_data_description_name_time_consistency(self):
599625
metadata_later_same_day = Metadata(
600626
name="Test Metadata",
601627
location="Test Location",
628+
subject=subject,
602629
data_description=data_description_later,
603630
acquisition=acquisition,
604631
)
@@ -620,6 +647,7 @@ def test_validate_data_description_name_time_consistency(self):
620647
metadata_next_day = Metadata(
621648
name="Test Metadata",
622649
location="Test Location",
650+
subject=subject,
623651
data_description=data_description_next_day,
624652
acquisition=acquisition,
625653
)
@@ -644,20 +672,25 @@ def test_validate_data_description_name_time_consistency(self):
644672
metadata_with_warning = Metadata(
645673
name="Test Metadata",
646674
location="Test Location",
675+
subject=subject,
647676
data_description=data_description_before,
648677
acquisition=acquisition,
649678
)
650679
# Should have successfully created the metadata object
651680
self.assertIsNotNone(metadata_with_warning)
652681
# Should have issued a warning
653-
self.assertEqual(len(warning_list), 1)
654-
self.assertIn("Creation time from data_description.name", str(warning_list[0].message))
655-
self.assertIn("should be close to the acquisition end time", str(warning_list[0].message))
682+
print(warning_list)
683+
# Filter to only the time consistency warning
684+
time_warnings = [w for w in warning_list if "Creation time from data_description" in str(w.message)]
685+
self.assertEqual(len(time_warnings), 1)
686+
self.assertIn("Creation time from data_description", str(time_warnings[0].message))
687+
self.assertIn("should be close to the acquisition end time", str(time_warnings[0].message))
656688

657689
# Test case where data_description is None (should pass)
658690
metadata_no_data_desc = Metadata(
659691
name="Test Metadata",
660692
location="Test Location",
693+
subject=subject,
661694
acquisition=acquisition,
662695
)
663696
self.assertIsNotNone(metadata_no_data_desc)
@@ -666,6 +699,7 @@ def test_validate_data_description_name_time_consistency(self):
666699
metadata_no_acquisition = Metadata(
667700
name="Test Metadata",
668701
location="Test Location",
702+
subject=subject,
669703
data_description=data_description,
670704
)
671705
self.assertIsNotNone(metadata_no_acquisition)

0 commit comments

Comments
 (0)