Skip to content

Conversation

Viicos
Copy link
Member

@Viicos Viicos commented Dec 10, 2024

Change Summary

Partly fixes #11033.
Requires pydantic/pydantic-core#1572

Related issue number

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

@github-actions github-actions bot added the relnotes-fix Used for bugfixes. label Dec 10, 2024
Copy link

cloudflare-workers-and-pages bot commented Dec 10, 2024

Deploying pydantic-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 008f6d9
Status: ✅  Deploy successful!
Preview URL: https://190a7bc0.pydantic-docs.pages.dev
Branch Preview URL: https://js-input-type-in-cs.pydantic-docs.pages.dev

View logs

@Viicos Viicos force-pushed the js-input-type-in-cs branch from cb8e796 to 212a9c5 Compare December 10, 2024 21:13
@@ -280,12 +280,37 @@ def handle_dict_schema(self, schema: core_schema.DictSchema, f: Walk) -> core_sc
schema['values_schema'] = self.walk(values_schema, f)
return schema

def handle_function_schema(self, schema: AnyFunctionSchema, f: Walk) -> core_schema.CoreSchema:
Copy link
Member Author

Choose a reason for hiding this comment

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

This method was in reality never used: there's no 'function' core schema. Instead, a method for each type was created.

schema['json_schema_input_schema'] = self.walk(schema['json_schema_input_schema'], f)
return schema

# TODO duplicate schema types for serializers and validators, needs to be deduplicated:
Copy link
Member Author

Choose a reason for hiding this comment

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

Will do in a follow up PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Great. Can you open an issue just so we can track?

Copy link
Member Author

Choose a reason for hiding this comment

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

@@ -637,29 +637,6 @@ def walk(s, recurse):
}


def test_handle_function_schema():
Copy link
Member Author

Choose a reason for hiding this comment

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

This test was added for coverage purposes, because as mentioned the method couldn't be called in "real" code

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.

Seems like a nice simplification, thanks for implementing this. Tells us, we shouldn't put core schemas in metadata.

I do think it's unfortunate that we have to store json schema related info on core schemas, but hey, they're coupled, so we might as well take advantage in v2.

schema['json_schema_input_schema'] = self.walk(schema['json_schema_input_schema'], f)
return schema

# TODO duplicate schema types for serializers and validators, needs to be deduplicated:
Copy link
Contributor

Choose a reason for hiding this comment

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

Great. Can you open an issue just so we can track?

@Viicos Viicos force-pushed the js-input-type-in-cs branch from 7e62daa to de23974 Compare December 11, 2024 12:12
Copy link

codspeed-hq bot commented Dec 11, 2024

CodSpeed Performance Report

Merging #11085 will not alter performance

Comparing js-input-type-in-cs (008f6d9) with main (d8b610c)

Summary

✅ 46 untouched benchmarks

Copy link
Contributor

github-actions bot commented Dec 11, 2024

Coverage report

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

@@ -229,13 +223,6 @@ def __get_pydantic_core_schema__(self, source_type: Any, handler: GetCoreSchemaH
serialization = None

input_schema = handler.generate_schema(self.json_schema_input_type)
Copy link
Member Author

Choose a reason for hiding this comment

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

Note to self: although the default json_schema_input_type value is Any, we should also account for PydanticUndefined if explicitly set.

@sydney-runkle sydney-runkle enabled auto-merge (squash) December 18, 2024 14:18
@sydney-runkle sydney-runkle merged commit 47ba4a5 into main Dec 18, 2024
52 checks passed
@sydney-runkle sydney-runkle deleted the js-input-type-in-cs branch December 18, 2024 14:20
Viicos added a commit that referenced this pull request Dec 18, 2024
Co-authored-by: Sydney Runkle <54324534+sydney-runkle@users.noreply.github.com>
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
Development

Successfully merging this pull request may close these issues.

JSON Schema gen chokes on BeforeValidator with a union in the json_schema_input_type
2 participants