dandi / dandi/dandi-schema

Improve `PydanticValidationError` to take a `pydantic.ValidationError` instead of a raw error-dict list

Open
#430 0 comments 0 reactions 1 assignee Claimed by @candleindark View on GitHub
Dominant language
Python
Stars
7
Forks
15
PR merge metrics
No merged PRs in 30d

Description

## Summary

`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

```py
class PydanticValidationError(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 #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`.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.