You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
PydanticValidationError.__init__ currently accepts errors: List[Dict[str, Any]], but the actual contract is narrower and undocumented: the list is assumed to be the pydantic_core.ErrorDetails list returned by pydantic.ValidationError.errors() (https://docs.pydantic.dev/latest/errors/errors/). Nothing in the type signature enforces this, so the constructor's real precondition is invisible to callers and to mypy.
Proposed change
classPydanticValidationError(ValidationError):
"""Validation errors were detected by pydantic"""def__init__(self, ve: pydantic.ValidationError) ->None:
self.errors: list[pydantic_core.ErrorDetails] =ve.errors()
Taking the raw pydantic.ValidationError makes the input contract explicit while leaving the constructed object unchanged: self.errors is still the same list[ErrorDetails] that consumers already read.
The same PR should also tighten the annotation on the receiving side in dandischema/tests/test_metadata.py (e.g. the comprehension over exc.value.errors around line 319) to pydantic_core.ErrorDetails, rather than leaving it as an untyped/Any value.
This should land after #422 merges, since that PR removes the missing_ok filtering in dandischema/metadata.py that currently sits between catching the exception and constructing PydanticValidationError; a constructor that recomputes ve.errors() internally can't reproduce that filtering.
Investigation details: sequencing rationale and cross-repo impact check
Sequencing: today, validate() catches pydantic.ValidationError, filters out "missing"-type entries when missing_ok=True, and only then builds PydanticValidationError from the filtered list. Once #422 removes missing_ok and that filtering branch, the call site becomes a straight pass-through of the caught exception, which is what this change assumes.
Cross-repo impact check (against local clones of dandi-cli and dandi-archive):
dandi-cli never imports or touches dandischema.exceptions at all. It catches pydantic.ValidationError directly and calls .errors() itself in dandi/files/bases.py (around lines 222 and 809-810), entirely independent of dandischema's exception wrapper. Unaffected by this change.
dandi-archive's dandiapi/api/services/metadata/__init__.py (_encode_pydantic_error, lines 28-49) only reads error.errors as a list of dicts with loc/msg keys. That is exactly the shape ve.errors() already produces, so no behavioral change is needed there.
dandi-schema's own tests (dandischema/tests/test_metadata.py, e.g. lines 98 and 319) index into .errors the same way, and the one test with different expectations, test_missing_ok, is removed by Remove the missing_ok parameter from metadata.validate() #422 anyway.
No cross-repo breakage is expected from this change.
Follow-up in dandi-archive
Once this change ships, dandi-archive's _encode_pydantic_error parameter (dandiapi/api/services/metadata/__init__.py:28) can be annotated as pydantic_core.ErrorDetails instead of an untyped/implicit dict. This has to be a follow-up PR in that repo rather than part of this one, since it lives outside dandi-schema.
Summary
PydanticValidationError.__init__currently acceptserrors: List[Dict[str, Any]], but the actual contract is narrower and undocumented: the list is assumed to be thepydantic_core.ErrorDetailslist returned bypydantic.ValidationError.errors()(https://docs.pydantic.dev/latest/errors/errors/). Nothing in the type signature enforces this, so the constructor's real precondition is invisible to callers and tomypy.Proposed change
Taking the raw
pydantic.ValidationErrormakes the input contract explicit while leaving the constructed object unchanged:self.errorsis still the samelist[ErrorDetails]that consumers already read.The same PR should also tighten the annotation on the receiving side in
dandischema/tests/test_metadata.py(e.g. the comprehension overexc.value.errorsaround line 319) topydantic_core.ErrorDetails, rather than leaving it as an untyped/Anyvalue.This should land after #422 merges, since that PR removes the
missing_okfiltering indandischema/metadata.pythat currently sits between catching the exception and constructingPydanticValidationError; a constructor that recomputesve.errors()internally can't reproduce that filtering.Investigation details: sequencing rationale and cross-repo impact check
Sequencing: today,
validate()catchespydantic.ValidationError, filters out"missing"-type entries whenmissing_ok=True, and only then buildsPydanticValidationErrorfrom the filtered list. Once #422 removesmissing_okand that filtering branch, the call site becomes a straight pass-through of the caught exception, which is what this change assumes.Cross-repo impact check (against local clones of
dandi-clianddandi-archive):dandi-clinever imports or touchesdandischema.exceptionsat all. It catchespydantic.ValidationErrordirectly and calls.errors()itself indandi/files/bases.py(around lines 222 and 809-810), entirely independent ofdandischema's exception wrapper. Unaffected by this change.dandi-archive'sdandiapi/api/services/metadata/__init__.py(_encode_pydantic_error, lines 28-49) only readserror.errorsas a list of dicts withloc/msgkeys. That is exactly the shapeve.errors()already produces, so no behavioral change is needed there.dandi-schema's own tests (dandischema/tests/test_metadata.py, e.g. lines 98 and 319) index into.errorsthe same way, and the one test with different expectations,test_missing_ok, is removed by Remove themissing_okparameter frommetadata.validate()#422 anyway.No cross-repo breakage is expected from this change.
Follow-up in
dandi-archiveOnce this change ships,
dandi-archive's_encode_pydantic_errorparameter (dandiapi/api/services/metadata/__init__.py:28) can be annotated aspydantic_core.ErrorDetailsinstead of an untyped/implicitdict. This has to be a follow-up PR in that repo rather than part of this one, since it lives outsidedandi-schema.