Skip to content

computed_field with MissingType in return type: PydanticSerializationError: Unable to serialize unknown type: <MissingType> #28

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

Open
thibaut-lo opened this issue Nov 5, 2024 · 8 comments

Comments

@thibaut-lo
Copy link

thibaut-lo commented Nov 5, 2024

Hi - this is a very nice lib! Just reporting something I saw

from pydantic_partials import PartialModel, MissingType, Missing

from pydantic import computed_field

class Test(PartialModel):
    a: int

    @computed_field
    @property
    def b(self) -> int | MissingType:
          if a < 2:
              return Missing
          return a

t = Test(a=1)
t.model_dump_json()

raises

.local/lib/python3.11/site-packages/pydantic/main.py", line 415, in model_dump_json
    return self.__pydantic_serializer__.to_json(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
pydantic_core._pydantic_core.PydanticSerializationError: Unable to serialize unknown type: <class 'pydantic_partials.sentinels.MissingType'>

Instead, I would expect it to drop the computed_fields.

This is nit.

Versions

pydantic-partials: 1.1.0
pydantic: 2.8.2

@joshorr
Copy link
Owner

joshorr commented Nov 12, 2024

@thibaut-lo Thank you for the report, very much appreciated!

I'll try and take a look at this by end of the week, see if I can get it fixed. I don't think I've tested the library with computed fields, so it's probably something I am not taking into account that I should have. I'll be sure to add a unit-test when I fix it.

Thank you!

@joshorr
Copy link
Owner

joshorr commented Nov 12, 2024

Thinking about it for a few minutes, it's probably related to the biggest limitation I have right now with Pedantic. I can't raise a PydanticOmit in a serializer without Pydantic producing an error (See pydantic/pydantic#5461 (reply in thread)).

I actually delete/prevent Missing values from being assigned to attributes right now as a work-around. A computed field however can return a value and I can't prevent it by deleting the attribute (since it's computed field property function).

When I get some time later this week to look into this, I'll have to see if I can figure out a workaround for this use-case, until there is some way to tell Pydantic not to serialize something from the serializer method.

@joshorr
Copy link
Owner

joshorr commented Nov 21, 2024

@thibaut-lo Sorry, I have not had a chance to see if there is something I can do to help this use case (had a family emergency over last weekend).

There might not be any good workaround, until PydanticOmit can be used in a serializer (see above). The Pydantic people seem interested in eventually doing that, but it's not a high priority for them at the moment.

Perhaps I can override the model_dump method and try to deal with it there. It won't fix model_dump_json, and I would have to recursively look the dump to find the Missing values and remove them.

So it's not ideal and a bit messy, but it may support your use-case well enough for now to be worth doing. Like I said, I'll see if I can do this later this week. We will see.

(I could use some field-based info to tell me where to look, but I would still have to look recursively in fields that have Pydantic types associated with them. Also, there are ways to rename stuff during export in various ways (alias, overriding model_dump in child class, etc), that I think I would have to look at all the values in the dict, sub-dicts, and lists to make sure I found all the Missing values.

@thibaut-lo
Copy link
Author

thibaut-lo commented Nov 27, 2024

@joshorr No worries, thanks a lot for having a look. 🙏

As a temporary handler for now, I have been overriding a bit the PartialModel to add a user configuration regarding what they want to get as behavior for serialization.

See below... Far from perfect. This doesn't overcome the issue (handling json serialization with Missing computed field) but at least provide a better ux around that flow.

from collections.abc import Callable
import logging
from typing import Any
from typing import Literal

from pydantic_core import PydanticSerializationError
from pydantic import SerializationInfo
from pydantic import model_serializer

from pydantic_partials import PartialModel as PydanticPartialModel
from pydantic_partials import PartialConfigDict as PydanticPartialConfigDict
from pydantic_partials import Missing
from pydantic_partials.meta import PartialMeta as PydanticPartialMeta

logger: logging.Logger = logging.getLogger(__name__)


class PartialConfigDict(PydanticPartialConfigDict):
    """
    Configuration dictionnary for partial models.
    """

    missing_fields_serialization: (
        Literal["forbid", "allow except json mode with missing computed fields"] | None
    ) = "forbid"
    """
    Configuration for missing fields in serialization.

    Defaults to "forbid".

    - If "forbid", the model raises an error when serializing if there are any fields or computed fields with Missing as value.

    - If "allow except json mode with missing computed fields", allow serialization with fields that have Missing as value, except if there are **computed** fields in the model that have Missing as value and the serialization mode is "json".

    Note that pydantic_partials does not support serialization of models with missing computed fields in json mode. See https://github.com/joshorr/pydantic-partials/issues/28
    """


class PartialMeta(PydanticPartialMeta):
    """
    Meta class for partial models.
    """

    config_dict: PartialConfigDict


class PartialModel(
    PydanticPartialModel,
    metaclass=PydanticPartialMeta,
    auto_partials=False,  # type: ignore # Do not set implicitely all fields as partial
):
    """
    Base class for partial models.
    """

    @property
    def has_missing_values(self) -> bool:
        """
        Returns:
            bool: True if and only if the model has missing values.
        """
        return len(self.model_partial_fields - set(self.__dict__.keys())) > 0

    @model_serializer(mode="wrap", when_used="always")
    def raise_if_serialization_with_missing_fields(
        self,
        handler: Callable[[Any], dict | str],
        info: SerializationInfo,
    ) -> dict | str:
        """
        Handles serialization with missing fields.

        When the output serialization is a dict, drop any field with Missing as value before serialization.

        Raises:
            ValueError: if serialization with missing fields or missing computed fields is forbidden by the configuration set for that model.

        Returns:
            dict | str: The serialized model, without missing fields if it is a dictionnary.
        """
        missing_computed_fields: set[str] = {
            field_name
            for field_name, field_info in self.model_computed_fields.items()
            if field_info.wrapped_property.fget  # fget may be None if property is write-only. Unhandled and does not exist in the current codebase
            and field_info.wrapped_property.fget(self) is Missing
        }

        match self.__class__.model_config.get("missing_fields_serialization", None):
            case "allow except json mode with missing computed fields":
                if missing_computed_fields and info.mode_is_json():
                    raise PydanticSerializationError(
                        f"Cannot serialize {self.__class__.__name__} with missing derived computed fields [{', '.join(missing_computed_fields)}]."
                    )
            case _ as missing_fields_serialization:
                if (
                    missing_fields_serialization
                    and missing_fields_serialization != "forbid"
                ):
                    logger.warning(
                        "The missing_fields_serialization value [%s] is not known. The default value 'forbid' is used.",
                        missing_fields_serialization,
                    )
                missing_fields: set[str] = {
                    field_name
                    for field_name in self.model_fields
                    if getattr(self, field_name) is Missing
                }
                if missing_computed_fields.union(missing_fields):
                    raise PydanticSerializationError(
                        f"Cannot serialize {self.__class__.__name__} with missing fields [{', '.join(missing_fields)}] and derived computed fields [{', '.join(missing_computed_fields)}]."
                    )

        dumped_self: Any = handler(self)

        if isinstance(dumped_self, dict):
            return {
                field_name: value
                for field_name, value in dumped_self.items()
                if value is not Missing
            }

        return dumped_self

@joshorr
Copy link
Owner

joshorr commented Nov 28, 2024

@thibaut-lo The good news is it sounds like there is a new feature in the next release of Pydantic (v2.11) that will allow one to conditionally exclude fields via a user-provided function. I could probably use this to do what is needed. As soon as the v2.11 is out, I'll attempt to utilize that new feature and get your use-case working.

The new features PR is here: pydantic/pydantic-core#1535

Today, I was actually going to try modifying the base-model and override model_dump to at least get that specific function working with your use case. But in light of this soon to be released feature in v2.11 of Pydantic, I'm going to wait for that and see if that new feature is good enough for your use case here.

Comments About Code

If it turns out that v2.11 can't be used for your use case, I'll seriously consider adding your code (with some tweaks) to the PartialModel.

Just a few side comments about the code you provided (in case it's helpful):

The hard thing about model_serializer is that the super-class's version won't be called if a subclass does its own model_serializer. This is one of the reasons I decided to not go this route in the first place. Also, there could be private fields (so the self.__dict__.keys() might include other attributes that don't matter). You would be better off using the ModelCls.model_fields property to get the Pydantic-aware fields and use that key-list instead.

@thibaut-lo
Copy link
Author

thibaut-lo commented Dec 9, 2024

Forgot to say thanks for both feedbacks! The incoming exclude-if pydantic feature sounds promising! Thanks again.

@joshorr joshorr mentioned this issue Jan 24, 2025
@joshorr
Copy link
Owner

joshorr commented Mar 12, 2025

Update: Still waiting on v2.11 of Pydantic. I am keeping an eye on it.

@joshorr
Copy link
Owner

joshorr commented Mar 31, 2025

@thibaut-lo Update: Pydantic v2.11 came out a couple of days ago. Unfortunately, it does not seem to have the exclude_if feature. It did seem like they were going to do it, as the feature is still on the Pydantic v2.11, and I see a comment on both PRs for the feature (in pydantic-core and main pydantic libraries). This is the comment for main lib PR about it: (pydantic/pydantic#10961 (comment)).

I left a comment on both PR's asking when it might come out. Sorry about the delay in finishing this feature 🙁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants