Properly wrap custom model_post_init implementation when private attributes are defined
#11251
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 #11251 will not alter performanceComparing Summary
|
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||
|
please review |
Viicos
requested changes
Jan 10, 2025
|
Thanks @Viicos - that is a much more elegant and correct solution! |
Viicos
requested changes
Jan 13, 2025
There was a problem hiding this comment.
Let's add an extra test following test_post_init:
def test_post_init_function_attrs_preserved() -> None:
class Model(BaseModel):
_a: int # Necessary to have model_post_init wrapped
def model_post_init(self, context, /) -> None:
"Custom docstring"""
assert Model.model_post_init.__doc__ = "Custom docstring"And revert the existing test changes.
There was a problem hiding this comment.
Thanks for the thorough review! Commit reverted, and new test added as requested.
model_post_init implementation when private attributes are defined
added 2 commits
January 14, 2025 12:04
Reverted after review This reverts commit 8ebf46c.
…`model_post_init` Requested in review
Viicos
approved these changes
Jan 14, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
The docstring of
model_post_initis overwritten with the docstring ofwrapped_model_post_init.In order to fix this, I just copy the docstring of model_post_init over to the new wrapped-object.
I also modified an existing test-case to test for this instead of creating a new one.
Related issue number
I think the change is quite small - so I didn't create an issue. I don't mind creating one if it is necessary :-)
Checklist
Selected Reviewer: @sydney-runkle