Do not compute JSON Schema default when plain serializers are used with `when_used` set to `'json-unless-none'` and the default value is `None` by Viicos · Pull Request #10121 · pydantic/pydantic

Viicos · 2024-08-13T10:02:50Z

Change Summary

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

cloudflare-workers-and-pages · 2024-08-13T10:04:18Z

Deploying pydantic-docs with Cloudflare Pages

Latest commit:	`eb1570b`
Status:	✅ Deploy successful!
Preview URL:	https://41c39d23.pydantic-docs.pages.dev
Branch Preview URL:	https://10118-plain-json-schema.pydantic-docs.pages.dev

View logs

codspeed-hq · 2024-08-13T10:09:30Z

CodSpeed Performance Report

Merging #10121 will not alter performance

_{Comparing 10118-plain-json-schema (eb1570b) with main (f0d8f65)}

Summary

✅ 17 untouched benchmarks

Viicos · 2024-08-13T10:15:01Z

+    assert Model.model_json_schema(mode='serialization') == {
+        'properties': {'custom_decimal': {'default': None, 'title': 'Custom Decimal', 'type': 'number'}},
+        'title': 'Model',
+        'type': 'object',
+    }


This revealed another issue with the JSON Schema: type should be 'anyOf': [{'type': 'number'}, {'type': 'null'}]}.

When the JSON Schema for our field is created, schema is of type default, and schema['schema'] is of type nullable:

pydantic/pydantic/json_schema.py

Lines 1022 to 1031 in 0c97049

def default_schema(self, schema: core_schema.WithDefaultSchema) -> JsonSchemaValue:

"""Generates a JSON schema that matches a schema with a default value.

Args:

schema: The core schema.

Returns:

The generated JSON schema.

"""

json_schema = self.generate_inner(schema['schema'])

However, self.generate_inner(schema['schema']) will not go through nullable_schema as we would expect. As there's a serialization key, ser_schema is being used:

pydantic/pydantic/json_schema.py

Lines 492 to 494 in 0c97049

if self.mode == 'serialization' and 'serialization' in schema_or_field:

ser_schema = schema_or_field['serialization'] # type: ignore

json_schema = self.ser_schema(ser_schema)

This makes sense, however, the issue is ser_schema kind of assumes when_used is always. I'll see if I can fix this in a follow up PR.

Viicos · 2024-08-13T11:26:41Z

+            and (ser_schema := schema['schema'].get('serialization'))
+            and (ser_func := ser_schema.get('function'))
+            and ser_schema.get('type') == 'function-plain'
+            and not ser_schema.get('info_arg')


Switching from and ser_schema.get('info_arg') is False releaved an issue in the tests, that I fixed in 0eed384.

…th `when_used` set to `'json-unless-none'` and the default value is `None`

github-actions · 2024-08-13T12:13:49Z

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
pydantic
config.py
json_schema.py
pydantic/_internal
_config.py
_model_construction.py
Project Total

_{This report was generated by python-coverage-comment-action}

sydney-runkle

This patch looks alright to me - I've left one comment that I think might simplify things!

sydney-runkle · 2024-08-13T15:16:08Z

+                # It might be that the provided default needs to be validated first
+                # (assuming `validate_default` is enabled). However, we can't perform
+                # such validation during JSON Schema generation so we don't support
+                # this pattern for now.
+                # (One example is when using `foo: ByteSize = '1MB'`, which validates and
+                # serializes as an int. In this case, `ser_func` is `int` and `int('1MB')` fails).
+                self.emit_warning(
+                    'non-serializable-default',
+                    f'Unable to serialize value {default!r} with the plain serializer; excluding default from JSON schema',
+                )
+                return json_schema


Could we do something like this?

pydantic/pydantic/json_schema.py

Lines 2017 to 2044 in 0c97049

def encode_default(self, dft: Any) -> Any:

"""Encode a default value to a JSON-serializable value.

This is used to encode default values for fields in the generated JSON schema.

Args:

dft: The default value to encode.

Returns:

The encoded default value.

"""

from .type_adapter import TypeAdapter, _type_has_config

config = self._config

try:

default = (

dft

if _type_has_config(type(dft))

else TypeAdapter(type(dft), config=config.config_dict).dump_python(dft, mode='json')

)

except PydanticSchemaGenerationError:

raise pydantic_core.PydanticSerializationError(f'Unable to encode default value {dft}')

return pydantic_core.to_jsonable_python(

default,

timedelta_mode=config.ser_json_timedelta,

bytes_mode=config.ser_json_bytes,

)

I feel like we shouldn't be skipping that logic here, we could maybe call that function?

I don't think it makes sense to do so, or we may end up with a wrong default value in the JSON Schema (in the related test fixed, '1MB' was set as the default value in the JSON Schema, but this isn't true as the default is 1000000). In my opinion, it's better to have no value than a wrong one.

However, we emit a warning here, so I'm ok to call encode_default.

That's fair - I wonder if we could use a similar pattern though + attempt to validate the default + include that in the JSON schema. Feels messy to do validation here, I admit.

I'm not opposed to the warning you have in place, I think that's good!

That's fair - I wonder if we could use a similar pattern though + attempt to validate the default + include that in the JSON schema. Feels messy to do validation here, I admit.

I think we can do this in a separate PR, if someone asks for it. I'm not keen to go ahead and do this now.

Change looks good to me, thanks for including the helpful warning.

…ma generation

sydney-runkle

Looks good @Viicos, nice work

sydney-runkle · 2024-08-14T15:08:00Z

+                # serializes as an int. In this case, `ser_func` is `int` and `int('1MB')` fails).
+                self.emit_warning(
+                    'non-serializable-default',
+                    f'Unable to serialize value {default!r} with the plain serializer; excluding default from JSON schema',


One nit, this isn't always a user defined plain serializer, which might be confusing. We could change the wording of this a bit, but I'm not super worried about it, so will go ahead and merge this.

github-actions Bot added the relnotes-fix Used for bugfixes. label Aug 13, 2024

Viicos commented Aug 13, 2024

View reviewed changes

Do not compute JSON Schema default when plain serializers are used wi…

4396bb3

…th `when_used` set to `'json-unless-none'` and the default value is `None`

Viicos force-pushed the 10118-plain-json-schema branch from acef83b to 4396bb3 Compare August 13, 2024 12:01

Viicos mentioned this pull request Aug 13, 2024

PlainSerializer not respecting when_used field during generation of json schema #10118

Closed

1 task

sydney-runkle suggested changes Aug 13, 2024

View reviewed changes

pydantic-hooky Bot added awaiting author revision awaiting changes from the PR author labels Aug 13, 2024

pydantic-hooky Bot assigned Viicos Aug 13, 2024

sydney-runkle mentioned this pull request Aug 13, 2024

pydantic weekly project plan (Sydney Runkle) - August 12th, 2024 #10109

Closed

25 tasks

Do not error when failing to serialize default value during JSON Sche…

eb1570b

…ma generation

Viicos force-pushed the 10118-plain-json-schema branch from 0eed384 to eb1570b Compare August 13, 2024 18:02

sydney-runkle approved these changes Aug 14, 2024

View reviewed changes

sydney-runkle reviewed Aug 14, 2024

View reviewed changes

sydney-runkle merged commit a42fb67 into main Aug 14, 2024

sydney-runkle deleted the 10118-plain-json-schema branch August 14, 2024 15:08

Viicos mentioned this pull request Aug 14, 2024

Handle nullable schemas with serialization schema available during JSON Schema generation #10132

Merged

5 tasks

zzstoatzz mentioned this pull request Aug 27, 2024

pydantic 2.9.0b1 breaks forward references in prefect PrefectHQ/prefect#15112

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Do not compute JSON Schema default when plain serializers are used with `when_used` set to `'json-unless-none'` and the default value is `None`#10121

Do not compute JSON Schema default when plain serializers are used with `when_used` set to `'json-unless-none'` and the default value is `None`#10121
sydney-runkle merged 2 commits intomainfrom
10118-plain-json-schema

Viicos commented Aug 13, 2024

Uh oh!

cloudflare-workers-and-pages Bot commented Aug 13, 2024 •

edited

Loading

Uh oh!

codspeed-hq Bot commented Aug 13, 2024 •

edited

Loading

Uh oh!

Viicos Aug 13, 2024

Uh oh!

Viicos Aug 13, 2024 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 13, 2024 •

edited

Loading

Uh oh!

sydney-runkle left a comment

Uh oh!

sydney-runkle Aug 13, 2024

Uh oh!

Viicos Aug 13, 2024

Uh oh!

sydney-runkle Aug 13, 2024

Uh oh!

sydney-runkle Aug 14, 2024

Uh oh!

sydney-runkle left a comment

Uh oh!

sydney-runkle Aug 14, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	def default_schema(self, schema: core_schema.WithDefaultSchema) -> JsonSchemaValue:
	"""Generates a JSON schema that matches a schema with a default value.

	Args:
	schema: The core schema.

	Returns:
	The generated JSON schema.
	"""
	json_schema = self.generate_inner(schema['schema'])

	if self.mode == 'serialization' and 'serialization' in schema_or_field:
	ser_schema = schema_or_field['serialization'] # type: ignore
	json_schema = self.ser_schema(ser_schema)

	def encode_default(self, dft: Any) -> Any:
	"""Encode a default value to a JSON-serializable value.

	This is used to encode default values for fields in the generated JSON schema.

	Args:
	dft: The default value to encode.

	Returns:
	The encoded default value.
	"""
	from .type_adapter import TypeAdapter, _type_has_config

	config = self._config
	try:
	default = (
	dft
	if _type_has_config(type(dft))
	else TypeAdapter(type(dft), config=config.config_dict).dump_python(dft, mode='json')
	)
	except PydanticSchemaGenerationError:
	raise pydantic_core.PydanticSerializationError(f'Unable to encode default value {dft}')

	return pydantic_core.to_jsonable_python(
	default,
	timedelta_mode=config.ser_json_timedelta,
	bytes_mode=config.ser_json_bytes,
	)

Uh oh!

Conversation

Viicos commented Aug 13, 2024

Change Summary

Related issue number

Checklist

Uh oh!

cloudflare-workers-and-pages Bot commented Aug 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploying pydantic-docs with Cloudflare Pages

Uh oh!

codspeed-hq Bot commented Aug 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Performance Report

Merging #10121 will not alter performance

Summary

Uh oh!

Viicos Aug 13, 2024

Choose a reason for hiding this comment

Uh oh!

Viicos Aug 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Aug 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage report

Uh oh!

sydney-runkle left a comment

Choose a reason for hiding this comment

Uh oh!

sydney-runkle Aug 13, 2024

Choose a reason for hiding this comment

Uh oh!

Viicos Aug 13, 2024

Choose a reason for hiding this comment

Uh oh!

sydney-runkle Aug 13, 2024

Choose a reason for hiding this comment

Uh oh!

sydney-runkle Aug 14, 2024

Choose a reason for hiding this comment

Uh oh!

sydney-runkle left a comment

Choose a reason for hiding this comment

Uh oh!

sydney-runkle Aug 14, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cloudflare-workers-and-pages Bot commented Aug 13, 2024 •

edited

Loading

codspeed-hq Bot commented Aug 13, 2024 •

edited

Loading

Viicos Aug 13, 2024 •

edited

Loading

github-actions Bot commented Aug 13, 2024 •

edited

Loading