AllenNeuralDynamics / aind-data-schema

A library that defines AIND data schema and validates JSON.
MIT License
23 stars 17 forks source link

Breaking changes in pydantic 2.9 (part 2) #1058

Closed dbirman closed 1 month ago

dbirman commented 1 month ago
======================================================================
FAIL: test_validate_ecephys_metadata (__main__.TestMetadata.test_validate_ecephys_metadata)
Tests that ecephys validator works as expected
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/dan/proj/aind/aind-data-schema/tests/test_metadata.py", line 189, in test_validate_ecephys_metadata
    self.assertIn(
AssertionError: 'Missing some metadata for Ecephys. Requires subject, procedures, session, rig, and processing.' not found in "1 validation error for Metadata\n  Value error, Expected 21 fields but got 14 for type `NanojectInjection` with value `NanojectInjection(recovery_time=None, recovery_tim...jection_volume_unit=<VolumeUnit.NL: 'nanoliter'>)` - serialized value may not be as expected. [type=value_error, input_value={'name': 'ecephys_655019_..._axes=None, notes=None)}, input_type=dict]\n    For further information visit https://errors.pydantic.dev/2.9/v/value_error"

======================================================================
FAIL: test_validate_smartspim_metadata (__main__.TestMetadata.test_validate_smartspim_metadata)
Tests that smartspim validator works as expected
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/dan/proj/aind/aind-data-schema/tests/test_metadata.py", line 148, in test_validate_smartspim_metadata
    self.assertIn(
AssertionError: 'Missing some metadata for SmartSpim. Requires subject, procedures, acquisition, and instrument.' not found in "1 validation error for Metadata\n  Value error, Expected 21 fields but got 14 for type `NanojectInjection` with value `NanojectInjection(recovery_time=None, recovery_tim...jection_volume_unit=<VolumeUnit.NL: 'nanoliter'>)` - serialized value may not be as expected. [type=value_error, input_value={'name': 'ecephys_655019_...oftware=[], notes=None)}, input_type=dict]\n    For further information visit https://errors.pydantic.dev/2.9/v/value_error"

These remaining issues are due to the use of model_construct() on classes with required fields (that then don't get generated. Something about pydantic 2.9 changed and these are sneaking past our attempt to ignore validation errors. Separating this from the issues Bruno fixed already

bruno-f-cruz commented 1 month ago

A future release may fix this https://github.com/pydantic/pydantic/issues/10344.

If time is short for fixing this may be worth pinning for a couple of weeks.

dbirman commented 1 month ago

A future release may fix this pydantic/pydantic#10344.

If time is short for fixing this may be worth pinning for a couple of weeks.

Okay finally had the time to look at what you had posted over there and I finally get the error. Thanks for posting that and getting it fixed (hopefully) upstream.

Another option would be to write a version of model_construct() for ourselves that creates required fields with None. I think it's just a matter of an else in this statement https://github.com/pydantic/pydantic/blob/204e109691c69583e656c6e16a62ad79da2f59b9/pydantic/main.py#L291C1-L291C39 that sets the value to None