pydantic / pydantic-core

Core validation logic for pydantic written in rust
MIT License
1.4k stars 232 forks source link

Try each option in union serializer before inference #1398

Closed sydney-runkle closed 1 month ago

sydney-runkle commented 1 month ago

Consider the following case:

from typing import Any
from pydantic import BaseModel, model_serializer

class Item(BaseModel):
    id: int

    @model_serializer
    def dump(self) -> dict[str, Any]:
        return {"id": self.id}

class ItemContainer(BaseModel):
    item_or_items: Item | list[Item]

def test_dump_json() -> None:
    items = [Item(id=i) for i in range(5)]
    ItemContainer(item_or_items=items).model_dump_json()

Previously, this used to error with:

pydantic_core._pydantic_core.PydanticSerializationError: Error serializing to JSON: PydanticSerializationError: Error calling function `dump`: AttributeError: 'list' object has no attribute 'id'

The problem here is that when attempting serialization against various union members, we immediately raised any error that wasn't an unexpected value error from pydantic-core (the one that @BoxyUwU just worked on improving).

This is inconsistent with our union validation logic, where we try validation against all union members, and swallow errors until all attempts fail, at which point we raise said errors.

With serialization in general, we're more relaxed, and often warn rather than error.

Now, with these changes, we collect any serialization errors and convert them to warnings if all serialization attempts fail. This helps to unify the logical patterns used for union validation and serialization, and enables more functional union serialization, especially in the case of custom serializers.

Fix https://github.com/pydantic/pydantic/issues/7642

codspeed-hq[bot] commented 1 month ago

CodSpeed Performance Report

Merging #1398 will not alter performance

Comparing union-ser-fallback-warnings (4090751) with main (863640b)

Summary

✅ 155 untouched benchmarks

BoxyUwU commented 1 month ago

would be nice for dh to take a look since im not confident here