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 serialization schema generation when using PlainValidator #10427

Merged
merged 3 commits into from
Sep 17, 2024
Merged

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.
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

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

Copy link
Contributor

github-actions bot commented Sep 17, 2024

Coverage report

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
Member

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
Member

@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!!

@sydney-runkle sydney-runkle enabled auto-merge (squash) September 17, 2024 14:21
@sydney-runkle sydney-runkle merged commit 00b3762 into main Sep 17, 2024
59 checks passed
@sydney-runkle sydney-runkle deleted the 10385 branch September 17, 2024 14:24
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
2 participants