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
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_field
with the same meanings as inField
to add info to the JSON schema.examples
solves the primary user request,json_schema_extra
was also mentioned in the issue comments.computed_field
docstring,json_schema_extra
inGenerateSchema
.Related issue number
Closes #7864
Checklist
Selected Reviewer: @Kludex