Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.2k
Add default factory with validated data to PrivateAttr#11685
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Add default factory with validated data to PrivateAttr#11685
Uh oh!
There was an error while loading.Please reload this page.
Conversation
codspeed-hqbot commentedApr 2, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
CodSpeed Performance ReportMerging#11685 willnot alter performanceComparing Summary
|
please review |
github-actionsbot commentedApr 2, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Coverage reportClick to see where and how coverage changed
This report was generated bypython-coverage-comment-action |
Viicos left a comment• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
Thanks for the contribution, this is looking good. Could you also addressmissing coverage for this PR?
Uh oh!
There was an error while loading.Please reload this page.
@@ -597,15 +597,6 @@ def deprecation_message(self) -> str | None: | |||
return 'deprecated' if self.deprecated else None | |||
return self.deprecated if isinstance(self.deprecated, str) else self.deprecated.message | |||
@property | |||
def default_factory_takes_validated_data(self) -> bool | None: |
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.
Although this was used inFieldInfo.get_default()
, this is meant to be used by users as well so we should keep it.
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 logic inside the new common method:_fields.resolve_default_value
--- Do you want to keep it as a property here and also in thePrivateAttr
?
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.
Yes please, and indeed it makes sense to have it onPrivateAttr
as well.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Sure, I will be updating it soon! Thanks |
shadycuz commentedApr 21, 2025
@Viicos This MR is now at 100% coverage, could you or someone else on the pydantic team please re-review it. I also need this fix. Thanks |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Victorien <65306057+Viicos@users.noreply.github.com>
Co-authored-by: Victorien <65306057+Viicos@users.noreply.github.com>
Co-authored-by: Victorien <65306057+Viicos@users.noreply.github.com>
Co-authored-by: Victorien <65306057+Viicos@users.noreply.github.com>
Co-authored-by: Victorien <65306057+Viicos@users.noreply.github.com>
4ce1ef3
tof426ad6
Compare@Viicos I already took care of your comments. |
Thanks, but it looks likehttps://github.com/pydantic/pydantic/pull/11685/files#r2056629172 wasn't addressed. |
oh ups, will be addressing this soon |
Uh oh!
There was an error while loading.Please reload this page.
Change Summary
This PR adds
default_factory
toPrivateAttr
working in a similar way that it works withField
. Due that private attributes happen after init, thedefault_factory
callable has available the entire__dict__
attribute. Also, private attributes definedbefore the current private are available (similar as this callable work with field)Related issue number
Fixes#10992
Checklist
Selected Reviewer:@sydney-runkle