-
Notifications
You must be signed in to change notification settings - Fork 219
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
Replace custom 'hidden=True' field attribute with builtin 'exclude=True' #741
Replace custom 'hidden=True' field attribute with builtin 'exclude=True' #741
Conversation
Hi, |
Restored backwards compatibility by transparently converting |
props[k] = v | ||
schema["properties"] = props | ||
# remove excluded fields from the json schema | ||
properties = schema.get("properties") |
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 don't remember how this call is implemented in Pydantic. Could you please check if the schema was not given as a mutable object here? If so, modifying it can lead to unexpected errors - it is better to clone it then.
I'll double-check this point from my side too.
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.
Not exactly sure what you mean, schema
is a mutable object (dict) as shown in the annotation. The json_schema_extra
callable returns None so it's only called for its side-effects, mutating schema. It would have no effect if the schema was cloned.
The calling code in Pydantic v2 is here: https://github.com/pydantic/pydantic/blob/main/pydantic/json_schema.py#L1364-L1368
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.
Yep, you are right. This is expected behavior.
612b405
to
437090e
Compare
Merged. It will be published tomorrow |
Thank you for the PR! |
Replace the custom
hidden
Pydantic Field attribute withexclude
and remove a bunch of code needed to support it.