Skip to content

Conversation

@Viicos
Copy link
Member

@Viicos Viicos commented Sep 17, 2024

Do not hardcode a new wrap serializer schema if a serialization schema was already generated before.

Note that the following can be used as a failing MRE, related to the TODO comment:

from typing import Annotated

from pydantic import BaseModel, PlainValidator
from pydantic_core import core_schema

class MyDict:
    @classmethod
    def __get_pydantic_core_schema__(cls, source, handler):
        return core_schema.dict_schema(
            keys_schema=handler.generate_schema(str),
            values_schema=handler.generate_schema(int),
            serialization=core_schema.filter_dict_schema(
                include={'a'},
            ),
        )


class Model(BaseModel):
    f: Annotated[MyDict, PlainValidator(val)]

# If core schema validation is disabled:
Model(f={'a': 1, 'b': 1}).model_dump()
#> {'f': {'a': 1, 'b': 1}}
# Should be: {'f': {'a': 1}}

Maybe we should create a new issue, and add a xfail test linking to this issue?

Related issue number

Fixes #10385
Fixes #8586

Checklist

  • The pull request title is a good summary of the changes - it will be used in the changelog
  • Unit tests for the changes exist
  • Tests pass on CI
  • Documentation reflects the changes where applicable
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

Do not hardcode a new wrap serializer schema if a serialization
schema was already generated before.
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Sep 17, 2024

Deploying pydantic-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 1fb016f
Status: ✅  Deploy successful!
Preview URL: https://956db31c.pydantic-docs.pages.dev
Branch Preview URL: https://10385.pydantic-docs.pages.dev

View logs

@codspeed-hq
Copy link

codspeed-hq bot commented Sep 17, 2024

CodSpeed Performance Report

Merging #10427 will not alter performance

Comparing 10385 (1fb016f) with main (cf671c8)

Summary

✅ 49 untouched benchmarks

@github-actions
Copy link
Contributor

github-actions bot commented Sep 17, 2024

Coverage report

Image Image

This PR does not seem to contain any modification to coverable code.

core_schema.wrap_serializer_function_ser_schema(
function=lambda v, h: h(v),
schema=schema,
return_schema=schema,
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we not need to do handler.generate_schema(source_type) here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hum I don't think it will change anything in this case as schema is guaranteed to not have any serialization key here, but I'll change to align with how it is done in Plain/WrapSerializer.

Copy link
Contributor

@sydney-runkle sydney-runkle left a comment

Choose a reason for hiding this comment

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

Wonderful, thanks @Viicos!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

relnotes-fix Used for bugfixes.

Projects

None yet

3 participants