Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix json serialization when computed field is excluded #1228

Merged
merged 2 commits into from
Mar 18, 2024

Conversation

sydney-runkle
Copy link
Member

@sydney-runkle sydney-runkle commented Mar 17, 2024

Fix pydantic/pydantic#9015

Previously, we were getting the value of a computed field before we checked if it was excluded, which could lead to a recursion error in a case like that shown below.

This mirrors the behavior of the to_python logic already present, which currently works.

from __future__ import annotations

import hashlib

from pydantic import BaseModel, computed_field


class MediaFileInfo(BaseModel):
    url: str
    duration: float
    bitrate: int
    format: str
    codec: str

    @computed_field(repr=False)
    def id(self) -> str:
        str_obj = self.model_dump_json(exclude={"id"})
        hash_obj = hashlib.sha256()
        hash_obj.update(str_obj.encode("utf-8"))
        return hash_obj.hexdigest()[:12]


class VideoMediaFileInfo(MediaFileInfo):
    width: int
    height: int


class AudioMediaFileInfo(MediaFileInfo):
    language: str


if __name__ == "__main__":
    mf = AudioMediaFileInfo(
        url="https://bla.com",
        duration=123,
        bitrate=123,
        format="mp4",
        codec="mp4",
        language="fr"
    )
    print(mf.id)

This case is now fixed, and prints the id as expected! I think it makes sense to add a test case in pydantic for this 👍

Selected Reviewer: @samuelcolvin

@sydney-runkle
Copy link
Member Author

Please review

Copy link

codspeed-hq bot commented Mar 17, 2024

CodSpeed Performance Report

Merging #1228 will not alter performance

Comparing fix_comp_field_ser_recursion (80c8f03) with main (8db3a6f)

Summary

✅ 148 untouched benchmarks

Copy link
Contributor

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 a test would be nice!

@sydney-runkle sydney-runkle merged commit b92a69b into main Mar 18, 2024
28 checks passed
@sydney-runkle sydney-runkle deleted the fix_comp_field_ser_recursion branch March 18, 2024 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RecursionError when serialising to JSON with computed field
3 participants