8000 Add default factory with validated data to PrivateAttr by andresliszt · Pull Request #11685 · pydantic/pydantic · GitHub
[go: up one dir, main page]
Skip to content

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Add default factory with validated data to PrivateAttr #11685

wants to merge 10 commits into from

Conversation

andresliszt
Copy link
Contributor
@andresliszt andresliszt commented Apr 2, 2025 •
edited
Loading

Change Summary

This PR adds default_factory to PrivateAttr working in a similar way that it works with Field. Due that private attributes happen after init, the default_factory callable has available the entire __dict__ attribute. Also, private attributes defined before the current private are available (similar as this callable work with field)

class Model(BaseModel):
    x: str
    _y: str = PrivateAttr(default = 'y')
    _z: str = PrivateAttr(default_factory = lambda data: data['x'] + data['_y'] + 'z')
    
m =  Model(x = 'x')
>>> m._z
'xyz'

# Fails with KeyError

class Model(BaseModel):
    x: str
    _y: str = PrivateAttr(default_factory= lambda data: data['x'] + data['_z'] + 'y')
    _z: str = PrivateAttr(default = 'z')

Model(x = 'x')
>>> KeyError

Related issue number

Fixes #10992

Checklist

  • The pull request title is a good summary of the changes - it will be used in the changelog
  • Unit tests for the changes exist
  • Tests pass on CI
  • Documentation reflects the changes where applicable
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

Selected Reviewer: @sydney-runkle

@github-actions github-actions bot added the relnotes-fix Used for bugfixes. label Apr 2, 2025
@codspeed-hq CodSpeed HQ
Copy link
codspeed-hq bot commented Apr 2, 2025 •
edited
Loading

CodSpeed Performance Report

Merging #11685 will not alter performance

Comparing andresliszt:add/validated-data-to-private-factory (312c1ab) with main (bf06866)

Summary

✅ 46 untouched benchmarks

@andresliszt
Copy link
Contributor Author

please review

@github-actions GitHub Actions
Copy link
Contributor
github-actions bot commented Apr 2, 2025 •
edited
Loading

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)		Lines missing
  pydantic
  fields.py
  pydantic/_internal
  _fields.py
  _model_construction.py
Project Total  

This report was generated by python-coverage-comment-action

Viicos
Viicos requested changes Apr 4, 2025
View reviewed changes
Copy link
Member
@Viicos Viicos left a comment •
edited
Loading

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 address missing coverage for this PR?

pydantic/fields.py
return self.deprecated if isinstance(self.deprecated, str) else self.deprecated.message

@property
def default_factory_takes_validated_data(self) -> bool | None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although this was used in FieldInfo.get_default(), this is meant to be used by users as well so we should keep it.

Copy link
Contributor Author

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 the PrivateAttr?

Copy link
Member

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 on PrivateAttr as well.

@Viicos Viicos added the awaiting author revision awaiting changes from the PR author label Apr 4, 2025
@andresliszt
Copy link
Contributor Author

Sure, I will be updating it soon! Thanks

@shadycuz
Copy link

@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

@Viicos Viicos added relnotes-feature and removed relnotes-fix Used for bugfixes. labels Apr 23, 2025
Viicos
Viicos reviewed Apr 23, 2025
View reviewed changes
Viicos
Viicos reviewed Apr 23, 2025
View reviewed changes
@Viicos Viicos mentioned this pull request Apr 24, 2025
Closed
13 tasks
andresliszt and others added 8 commits April 27, 2025 10:00
@andresliszt
Add default factory with validated data to PrivateAttr
b079644
@Viicos @andresliszt
Update pydantic/fields.py
1552766 
Co-authored-by: Victorien <65306057+Viicos@users.noreply.github.com>
@Viicos @andresliszt
Update tests/test_private_attributes.py
44edadb 
Co-authored-by: Victorien <65306057+Viicos@users.noreply.github.com>
@Viicos @andresliszt
Update tests/test_private_attributes.py
14810ac 
Co-authored-by: Victorien <65306057+Viicos@users.noreply.github.com>
@Viicos @andresliszt
Update tests/test_private_attributes.py
5f4503e 
Co-authored-by: Victorien <65306057+Viicos@users.noreply.github.com>
@Viicos @andresliszt
Update pydantic/fields.py
77a9cb1 
Co-authored-by: Victorien <65306057+Viicos@users.noreply.github.com>
@andresliszt
Update tests
d8caffe
@andresliszt
improve docs
f426ad6
@andresliszt andresliszt force-pushed the add/validated-data-to-private-factory branch from 4ce1ef3 to f426ad6 Compare April 27, 2025 14:05
@andresliszt
Copy link
Contributor Author

@Viicos I already took care of your comments.

@Viicos
Copy link
Member
Viicos commented Apr 30, 2025

@Viicos I already took care of your comments.

Thanks, but it looks like https://github.com/pydantic/pydantic/pull/11685/files#r2056629172 wasn't addressed.

@andresliszt
Copy link
Contributor Author

@Viicos I already took care of your comments.

Thanks, but it looks like https://github.com/pydantic/pydantic/pull/11685/files#r2056629172 wasn't addressed.

oh ups, will be addressing this soon

@andresliszt
Copy link
Contributor Author
andresliszt commented Jul 23, 2025 •
edited
Loading

@Viicos is this still needed ? I have some time these days to address your comments. Sorry for the delay

@Viicos
Copy link
Member
Viicos commented Jul 24, 2025

I think it is still relevant yes, feel free to address the remaining comments!

@TimothyWillard TimothyWillard mentioned this pull request Aug 22, 2025
Open
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Viicos Viicos Viicos requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
awaiting author revision awaiting changes from the PR author relnotes-feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

PrivateAttr's default_factory should support taking validated data as an argument, like Field does as of Pydantic 2.10
3 participants
@andresliszt @shadycuz @Viicos
0