model_json_schema export with Annotated types misses 'required' parameters #8793
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
CodSpeed Performance ReportMerging #8793 will not alter performanceComparing Summary
|
|
please review First open source contribution 🤞 |
sydney-runkle
suggested changes
Feb 13, 2024
tests/test_json_schema.py
Outdated
|
|
||
|
|
||
| def test_json_schema_annotated_with_field() -> None: | ||
| """Check if the ellipsis in the signature is considered as a required field.""" |
There was a problem hiding this comment.
Suggested change
| """Check if the ellipsis in the signature is considered as a required field.""" | |
| """Ensure field specified with Annotated in create_model call is still marked as required.""" |
|
|
||
| Model = create_model( | ||
| 'test_model', | ||
| bar=(Annotated[int, Field(description='Bar description')], ...), |
There was a problem hiding this comment.
Could you include this example as well? baz=(Annotated[int, Field(..., description="Baz description")], ...),
There was a problem hiding this comment.
Maybe with this model:
Model = create_model(
'test_model',
foo=(int, ...),
bar=(Annotated[int, Field(description="Bar description")], ...),
baz=(Annotated[int, Field(..., description="Baz description")], ...),
)
pydantic-hooky
bot
added
awaiting author revision
|
So, with a slight modification to the logic in your PR, we can actually fix many related issues. My proposal is the following: Which fixes:
Would you be willing to add a test for the second issue as well? Thanks for your contribution! |
if field_info.default is not PydanticUndefined and default_override is not PydanticUndefined:
field_info.default = default_overrideI think this was the wrong proposal because if a field is initialized with a default = PydanticUndefined, the overide won't work 😄 My modification: if default_override is not PydanticUndefined:
field_info.default = default_override |
dmontagu
approved these changes
Feb 13, 2024
sydney-runkle
approved these changes
Feb 13, 2024
|
Looks great, thanks for the help with the fix! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Change Summary
When using an Annotation with a FieldInfo and an Ellipsis, the Ellipsis was considered as a default value. Therefore, the field was not considered mandatory.
Related issue number
fix #8780
fix #8634
Checklist
Selected Reviewer: @lig