Skip to content

Fix: Properly update private attrs with model_copy, update= and extra=allow #12117

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
strawgate wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Fix: Properly update private attrs with model_copy, update= and extra=allow #12117

strawgate wants to merge 4 commits into from

Conversation

@strawgate
Copy link

@strawgate strawgate commented Aug 3, 2025 •
edited by pydantic-hooky bot
Loading

Change Summary

When using model_copy's update feature, private attributes are not correctly updated if extra=allow.

Related issue number

fix #12116

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: @Viicos

@github-actions github-actions bot added the relnotes-fix Used for bugfixes. label Aug 3, 2025
@strawgate strawgate mentioned this pull request Aug 3, 2025
Closed
@github-actions
Copy link
Contributor

github-actions bot commented Aug 3, 2025 •
edited
Loading

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)		Lines missing
  pydantic
  main.py 413
Project Total  

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

@codspeed-hq
Copy link

codspeed-hq bot commented Aug 3, 2025 •
edited
Loading

CodSpeed Performance Report

Merging #12117 will not alter performance

Comparing strawgate:fix_model_copy_private_attr (2dcb1ff) with main (f7a9b73)

Summary

✅ 46 untouched benchmarks

@strawgate strawgate changed the title Fix: Properly copy private attrs with model_copy, update= and extra=allow Fix: Properly update private attrs with model_copy, update= and extra=allow Aug 3, 2025
@strawgate
Copy link
Author

strawgate commented Aug 3, 2025

please review

Viicos
Viicos reviewed Aug 14, 2025
View reviewed changes
Copy link
Member

@Viicos Viicos left a comment

Choose a reason for hiding this comment

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

In the case of extra="forbid" and extra="ignore", I think this was only accidentally working. In fact, things get inconsistent currently:

copy_working = original_working.model_copy(update={"_private": "private_copy"})
copy_working._private
#> private_copy
copy_working.__pydantic_private__
#> {'_private': 'private default'}

Could you please update the PR to also take these cases into account? I think we need to also take into account private attributes for all extra cases.

But thinking about it, I'm afraid this will only be possible to do in v3 Under consideration for V3 . Users might be relying on providing private fields names in model_copy() to be included in the extra fields.

@pydantic-hooky pydantic-hooky bot added awaiting author revision awaiting changes from the PR author and removed ready for review labels Aug 14, 2025
@pydantic-hooky pydantic-hooky bot assigned strawgate and unassigned Viicos Aug 14, 2025
@Viicos Viicos added the deferred Deferred until future release or until something else gets done label Aug 14, 2025
@strawgate
Copy link
Author

strawgate commented Aug 17, 2025

Could you please update the PR to also take these cases into account? I think we need to also take into account private attributes for all extra cases.

Yep, done.

Users might be relying on providing private fields names in model_copy() to be included in the extra fields.

Could we change the behavior to continue to set the extra fields so that we fix the reported issue and maintain compatibility and have the v3 change be to remove the setting the extra fields?

@Viicos
Copy link
Member

Viicos commented Aug 18, 2025

Could we change the behavior to continue to set the extra fields so that we fix the reported issue and maintain compatibility and have the v3 change be to remove the setting the extra fields?

Do you have an example of how that would look like? I'm not sure I understand exactly what would be changed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Viicos Viicos Viicos left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@strawgate strawgate

Labels

awaiting author revision awaiting changes from the PR author deferred Deferred until future release or until something else gets done relnotes-fix Used for bugfixes.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

model_copy(update=...) with extra=allow places privateattr in extra

2 participants

@strawgate @Viicos