-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Add examples and json_schema_extra to @computed_field
#8013
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
Add examples and json_schema_extra to @computed_field
#8013
Conversation
| repr: bool = True, | ||
| return_type: Any = PydanticUndefined, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved this so that the overload and the actual signature matched better so they were easier to compare. Then I noticed that here it's repr: bool = True but the actual signature has repr: bool | None = None, is that intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question.
I don't think so. I think we should use repr: bool | None = None here as well. As far as I understand it, the reason we allow None here is so that we can enforce a default of True if the computed_field is public and False if it's not.
examples and json_schema_extra to @computed_field
|
please review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM otherwise. Thanks for factoring out the common logic re the get_json_schema_update_func and add_json_schema_extra. Definitely cleaner now.
| repr: bool = True, | ||
| return_type: Any = PydanticUndefined, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question.
I don't think so. I think we should use repr: bool | None = None here as well. As far as I understand it, the reason we allow None here is so that we can enforce a default of True if the computed_field is public and False if it's not.
|
I'm going to go ahead and merge this, thanks for your work on it. I opened an issue to add some better docs and type hints, as we've discussed in this PR. |
Change Summary
computed_fieldwith the same meanings as inFieldto add info to the JSON schema.examplessolves the primary user request,json_schema_extrawas also mentioned in the issue comments.computed_fielddocstring,json_schema_extrainGenerateSchema.Related issue number
Closes #7864
Checklist
Selected Reviewer: @Kludex