allow for shallow copies #4093

timkpaine · 2022-05-19T18:29:01Z

Change Summary

Related issue number

Fix #4092.

Checklist

Unit tests for the changes exist
Tests pass on CI and coverage remains at 100%
Documentation reflects the changes where applicable
changes/<pull request or issue id>-<github username>.md file added describing change
(see changes/README.md for details)
My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

timkpaine · 2022-05-19T18:47:18Z

please review

samuelcolvin · 2022-05-19T18:48:37Z

Thanks so much. I'm just moving house at the moment, but hopefully will find time to review this in the next few days.

timkpaine · 2022-05-19T18:51:37Z

@samuelcolvin no worries! pinning 1.9.0 is a workaround for now (or setting Config.copy_on_model_validation = False, though the behavior is somewhat different).

pydantic/config.py

samuelcolvin · 2022-05-20T13:19:31Z

You need to change the base for this PR to 1.9.X-fixes, not master.

samuelcolvin

Also need to add something to the documentation.

pydantic/config.py

pydantic/main.py

samuelcolvin · 2022-08-08T15:31:52Z

At this stage, I think most people have worked around this, I think best we leave behaviour as it is.

AngellusMortis · 2022-08-08T15:33:59Z

We have not worked around it in Home Assistant. Instead, we blacklisted this release since it is a breaking change. Introducing breaking changes in a bugfix revision and then not fixing them is not a good way to handle versioning.

samuelcolvin · 2022-08-08T15:48:33Z

I understand this isn't ideal @AngellusMortis, the breaking change wasn't intentional.

As far as I can tell, this change wasn't a simple fix for the original problem, it introduced new settings, also as you mentioned it introduced a new breaking change.

If you (or anyone else) can create a fix for the issue, I'll happily review it and try to get this fixed. Otherwise I'll try to get to it soon.

AngellusMortis · 2022-08-08T15:52:19Z

Even if this PR is canceled, as long as it is fixed is the important part. Introducing a breaking change in a bugfix versions and then not fixing it erodes the trust in the package. It would be good enough reason for me to consider an alternative solution as I do not want to use a package whose response to semver is "oh there was a breaking change, oh well, I am sure people will work around it".

timkpaine · 2022-08-08T15:56:49Z

If you (or anyone else) can create a fix for the issue

This PR was a compromise solution that kept the breaking change and added in extra functionality to allow users to revert to the old behavior. Simply changing the functionality back will itself be a breaking change (no doubt released in a patch version), so I recommend not doing this and going with this PR instead.

samuelcolvin · 2022-08-08T15:57:26Z

I understand this isn't ideal

Again, I'm really sorry about this. Finding time to work on pydantic when I'm not paid by a big company to do so has been hard, hence I've had to prioritise.

Since I've been working on it full time, I've been mostly concentrating on preparing for V2. One of the big reason to rewrite pydantic from the ground up in V2 is to make the codebase easier to work with so we can reduce the chance of breaking changes in minor or patch releases. Again, we try to avoid breaking changes as much as possible.

samuelcolvin · 2022-08-08T21:03:32Z

Ok, please comment if there's anything you think needs changing and I'll try to merge tomorrow and get a new release out. Is there anything else which should go in v1.9.2?

…

On Mon, 8 Aug 2022, 22:01 Eric Jolibois, ***@***.***> wrote: FWIW @samuelcolvin <https://github.com/samuelcolvin> I agree with @timkpaine <https://github.com/timkpaine> that this PR has the right approach now (it is what I would have done #4093 (comment) <#4093 (comment)>) In v2 we'll go with explicit calls of copy or deepcopy but in the meantime having this literal is probably the best to be 1.9.0 compliant — Reply to this email directly, view it on GitHub <#4093 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA62GGNINKQ273N6OKWTRFLVYFYRZANCNFSM5WNCJPPA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

peterschutt · 2022-08-09T10:19:17Z

In preparation for both V1.10 and V2 it would be good to have a list of open source projects which use pydantic where we can test functionality and thereby try to avoid more breaking changes?

One for your list - starlite is an API framework that heavily leverages pydantic.

samuelcolvin · 2022-08-09T13:22:12Z

Okay I think this is fixed and ready to be deployed.

@AngellusMortis @PrettyWood please can you review and confirm you're happy?

samuelcolvin · 2022-08-09T15:14:39Z

In preparation for both V1.10 and V2 it would be good to have a list of open source projects which use pydantic where we can test functionality and thereby try to avoid more breaking changes?

see #4359, moving discussion to there.

PrettyWood

Except the default value and the note looks good! Thank you!

PrettyWood · 2022-08-10T11:47:34Z

changes/4093-timkpaine.md

@@ -0,0 +1,3 @@
+Allow for shallow copies of attributes, adjusting the behavior of #3642
+`Config.copy_on_model_validation` is now a str enum of `["none", "deep", "shallow"]` corresponding to 
+not copying, deep copy, shallow copy.


My main concern is on the default value

With the test example

class User(BaseModel): hobbies: list[str] my_user = User(hobbies=['scuba diving']) class Transaction(BaseModel): user: User t = Transaction(user=my_user) print(t.user is my_user) print(t.user.hobbies is my_user.hobbies)

we used to have in v1.9.0

copy_on_model_validation = True (default): False, True copy_on_model_validation = False: True, True

which got changed (the famous breaking change) in 1.9.1

copy_on_model_validation = True (default): False, False <-💥 copy_on_model_validation = False: True, True

We now have with this patch

copy_on_model_validation = 'deep' : False, False copy_on_model_validation = 'shallow': False, True copy_on_model_validation = 'none': True, True

I wonder if we should write a note saying a breaking change has been introduced by mistake in a release version and that the old 1.9.0 behaviour has been fixed. In which case, the default value should be 'shallow'.

I agree, we should make the default shallow.

I'll try to fix this today.

default changed.

PrettyWood

LGTM! Thanks a lot
@samuelcolvin we'll need to add a note at the start of the release note for this issue and patch

samuelcolvin · 2022-08-10T18:06:06Z

Thanks for the review, will do.

I'll do the release tomorrow.

* allow for shallow copies * Add changes file * tweak change * update for comments * rename attr * use single quotes * bump ci * add warning if not a string, switch to string literals * fix linting, prompt ci * fix ci * extend and fix tests * change default to "shallow" Co-authored-by: Samuel Colvin <s@muelcolvin.com>

@Rabscuttler

* generate history from changes, uprev * Pydantic V2 blog (#4218) * first draft of pydantic V2 blog * more blog * blog rendering and formatting * more section * completing conversion table * prompt build * reviewing blog post * more reviewing and extending * recommendations from @Rabscuttler and @PrettyWood * add implementation details and more suggestions * comment about breaking changes * convert namespae to table, more removals * Apply suggestions from code review by @tiangolo Co-authored-by: Sebastián Ramírez <tiangolo@gmail.com> * feedback from @tiangolo's review * changes from @adriangb's review * Apply suggestions from code review Co-authored-by: Zac Hatfield-Dodds <zac.hatfield.dodds@gmail.com> * convert namespace info to psuedo-code * rename property, remove schema_json() * adding validation context * remove 'model_schema_json', take 2 * more tweaks while reviewing * comment about pypy and tagged unions * add thanks :prey:, prepare for release * suggestions from @PrettyWood * suggestions from @PrettyWood, model_dump_json comment Co-authored-by: Sebastián Ramírez <tiangolo@gmail.com> Co-authored-by: Zac Hatfield-Dodds <zac.hatfield.dodds@gmail.com> * comments mostly from @PrettyWood (#4226) * comments mostly from @PrettyWood * add updated comment * fix pre-commit * allow for shallow copies (#4093) * allow for shallow copies * Add changes file * tweak change * update for comments * rename attr * use single quotes * bump ci * add warning if not a string, switch to string literals * fix linting, prompt ci * fix ci * extend and fix tests * change default to "shallow" Co-authored-by: Samuel Colvin <s@muelcolvin.com> * uprev and prepare for release * linting Co-authored-by: Sebastián Ramírez <tiangolo@gmail.com> Co-authored-by: Zac Hatfield-Dodds <zac.hatfield.dodds@gmail.com> Co-authored-by: Tim Paine <t.paine154@gmail.com>

NunchakusLei · 2022-08-19T14:58:02Z

Looks like this PR reproduce the issue #3641

from pydantic import BaseModel
from typing import List


class A(BaseModel):
    name: str
        

class B(BaseModel):
    list_a: List[A]


a = A(name="a")
b = B(list_a=[a])

print(a)
print(b)
a.name = "b"  # make a change to variable a's field
print(a)
print(b)

Output in 1.6.2, 1.7.4, 1.8.1, 1.8.2, and 1.9.1,

A(name='a')
B(list_a=[A(name='a')])
A(name='b')
B(list_a=[A(name='a')])

Output in 1.9.0 and 1.9.2,

A(name='a')
B(list_a=[A(name='a')])
A(name='b')
B(list_a=[A(name='b')])

Is this the expected change of behaviour from previous version to v1.9.x?

timkpaine · 2022-08-19T15:23:33Z

@NunchakusLei yes, this PR restores state to some combination of 1.9.0 and 1.9.1 behavior, but lets you pick between them.

from the docs:

Allow for shallow copies of model fields, Config.copy_on_model_validation is now a str which must be 'none', 'deep', or 'shallow' corresponding to not copying, deep copy & shallow copy; default 'shallow',

So you'll want deep to restore the (erroneously changed) 1.9.1 behavior

samuelcolvin · 2022-08-19T15:26:14Z

See #4369, I need to update the docs.

NunchakusLei · 2022-08-19T15:56:03Z

@NunchakusLei yes, this PR restores state to some combination of 1.9.0 and 1.9.1 behavior, but lets you pick between them.

from the docs:
Allow for shallow copies of model fields, Config.copy_on_model_validation is now a str which must be 'none', 'deep', or 'shallow' corresponding to not copying, deep copy & shallow copy; default 'shallow',
So you'll want deep to restore the (erroneously changed) 1.9.1 behavior

Do you know if the 1.8.2 and previous are equivalent to deep behaviour? That's the version behaviour I try to restore to.

timkpaine · 2022-08-19T16:02:55Z

pre 1.9.1 was default shallow or "none", 1.9.1 was default deep or none, 1.9.2 is none/deep/shallow

NunchakusLei · 2022-08-19T16:50:25Z

@timkpaine Base on my test #4093 (comment), pre 1.9.0 was default deep or none (tested in 1.6.2, 1.7.4, 1.8.1, and 1.8.2).

The test result conflicting the statement of,

pre 1.9.1 was default shallow or "none" ...

I got very confuse now so want to clarify.

samuelcolvin · 2022-08-19T17:34:18Z

On my phone but I think:

Pre 1.9.0: default SHALLOW, config setting for NONE
1.9.0, 1.9.1: default DEEP, config setting for NONE
1.9.2: default SHALLOW, config settings for NONE or DEEP

NunchakusLei · 2022-08-19T18:10:55Z

@samuelcolvin

Pre 1.9.0: default SHALLOW, config setting for NONE

This doesn't matched my test result in 1.6.2, 1.7.4, 1.8.1, and 1.8.2. #4093 (comment)

timkpaine · 2022-08-19T20:03:16Z

@NunchakusLei your tests results might be wrong, but also...it doesnt really matter at this point, all behaviors are supported now.

samuelcolvin · 2022-08-22T20:41:27Z

@samuelcolvin

Pre 1.9.0: default SHALLOW, config setting for NONE

This doesn't matched my test result in 1.6.2, 1.7.4, 1.8.1, and 1.8.2. #4093 (comment)

See: https://github.com/pydantic/pydantic/blob/v1.8.2/pydantic/main.py#L731 pretty sure I'm correct.

efisoft-elt · 2023-02-24T14:09:17Z

class M(BaseModel):
    class Config:
        copy_on_model_validation = "!! Some non sens str !!"

Does not raise any error.

This is probably due to the fact that deprecation warning is handled.

            if copy_on_model_validation not in {'deep', 'shallow', 'none'}:
                # Warn about deprecated behavior
                warnings.warn(
                    "`copy_on_model_validation` should be a string: 'deep', 'shallow' or 'none'", DeprecationWarning
                )
                if copy_on_model_validation:
                    deep_copy = False

timkpaine added 3 commits May 19, 2022 14:28

allow for shallow copies

54f5e31

Add changes file

49461f3

tweak change

a1690d1

timkpaine mentioned this pull request May 19, 2022

1.9.1 Breaking change with model deep copying #4092

Closed

3 tasks

github-actions bot added the ready for review label May 19, 2022

github-actions bot assigned PrettyWood and samuelcolvin May 19, 2022

PrettyWood reviewed May 20, 2022

View reviewed changes

pydantic/config.py Outdated Show resolved Hide resolved

timkpaine added 2 commits May 20, 2022 09:29

update for comments

556231f

rename attr

f8a2705

timkpaine changed the base branch from master to 1.9.X-fixes May 20, 2022 13:33

timkpaine added 2 commits May 20, 2022 09:33

use single quotes

4eeb6df

bump ci

64bf6f8

Segelzwerg mentioned this pull request May 20, 2022

Build(deps): bump pydantic from 1.9.0 to 1.9.1 Whist-Team/Whist-Core#193

Closed

samuelcolvin reviewed May 23, 2022

View reviewed changes

pydantic/config.py Outdated Show resolved Hide resolved

pydantic/main.py Outdated Show resolved Hide resolved

timkpaine added 2 commits May 23, 2022 09:38

add warning if not a string, switch to string literals

bb33a04

Merge branch '1.9.X-fixes' into tkp/shallow

ce26312

AngellusMortis reviewed Jun 1, 2022

View reviewed changes

pydantic/main.py Outdated Show resolved Hide resolved

samuelcolvin closed this Aug 8, 2022

samuelcolvin reopened this Aug 8, 2022

samuelcolvin added 3 commits August 9, 2022 12:40

fix linting, prompt ci

c055976

fix ci

0b998a5

extend and fix tests

c5da8e2

samuelcolvin mentioned this pull request Aug 9, 2022

1.9.x #4076

Closed

11 tasks

PrettyWood reviewed Aug 10, 2022

View reviewed changes

change default to "shallow"

cc39d7f

PrettyWood approved these changes Aug 10, 2022

View reviewed changes

samuelcolvin merged commit 1d6a6e6 into pydantic:1.9.X-fixes Aug 10, 2022

timkpaine deleted the tkp/shallow branch August 19, 2022 15:23

timkpaine mentioned this pull request Aug 19, 2022

copy_on_model_validation config doesn't copy List field anymore in v1.9.0 #3641

Closed

3 tasks

glenrobson mentioned this pull request Nov 11, 2022

Create canvas from IIIF Image iiif-prezi/iiif-prezi3#89

Merged

koxudaxi mentioned this pull request Nov 21, 2022

Fix deprecation warnings around "copy_on_model_validation" koxudaxi/datamodel-code-generator#927

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

allow for shallow copies #4093

allow for shallow copies #4093

timkpaine commented May 19, 2022 •

edited by samuelcolvin

timkpaine commented May 19, 2022

samuelcolvin commented May 19, 2022

timkpaine commented May 19, 2022

samuelcolvin commented May 20, 2022

samuelcolvin left a comment

samuelcolvin commented Aug 8, 2022

AngellusMortis commented Aug 8, 2022 •

edited

samuelcolvin commented Aug 8, 2022

AngellusMortis commented Aug 8, 2022 •

edited

timkpaine commented Aug 8, 2022 •

edited

samuelcolvin commented Aug 8, 2022

samuelcolvin commented Aug 8, 2022 via email

peterschutt commented Aug 9, 2022

samuelcolvin commented Aug 9, 2022

samuelcolvin commented Aug 9, 2022

PrettyWood left a comment

PrettyWood Aug 10, 2022

samuelcolvin Aug 10, 2022

samuelcolvin Aug 10, 2022

PrettyWood left a comment

samuelcolvin commented Aug 10, 2022

NunchakusLei commented Aug 19, 2022

timkpaine commented Aug 19, 2022 •

edited

samuelcolvin commented Aug 19, 2022

NunchakusLei commented Aug 19, 2022 •

edited

timkpaine commented Aug 19, 2022

NunchakusLei commented Aug 19, 2022

samuelcolvin commented Aug 19, 2022

NunchakusLei commented Aug 19, 2022

timkpaine commented Aug 19, 2022

samuelcolvin commented Aug 22, 2022

efisoft-elt commented Feb 24, 2023 •

edited

allow for shallow copies #4093

allow for shallow copies #4093

Conversation

timkpaine commented May 19, 2022 • edited by samuelcolvin

Change Summary

Related issue number

Checklist

timkpaine commented May 19, 2022

samuelcolvin commented May 19, 2022

timkpaine commented May 19, 2022

samuelcolvin commented May 20, 2022

samuelcolvin left a comment

Choose a reason for hiding this comment

samuelcolvin commented Aug 8, 2022

AngellusMortis commented Aug 8, 2022 • edited

samuelcolvin commented Aug 8, 2022

AngellusMortis commented Aug 8, 2022 • edited

timkpaine commented Aug 8, 2022 • edited

samuelcolvin commented Aug 8, 2022

samuelcolvin commented Aug 8, 2022 via email

peterschutt commented Aug 9, 2022

samuelcolvin commented Aug 9, 2022

samuelcolvin commented Aug 9, 2022

PrettyWood left a comment

Choose a reason for hiding this comment

PrettyWood Aug 10, 2022

Choose a reason for hiding this comment

samuelcolvin Aug 10, 2022

Choose a reason for hiding this comment

samuelcolvin Aug 10, 2022

Choose a reason for hiding this comment

PrettyWood left a comment

Choose a reason for hiding this comment

samuelcolvin commented Aug 10, 2022

NunchakusLei commented Aug 19, 2022

timkpaine commented Aug 19, 2022 • edited

samuelcolvin commented Aug 19, 2022

NunchakusLei commented Aug 19, 2022 • edited

timkpaine commented Aug 19, 2022

NunchakusLei commented Aug 19, 2022

samuelcolvin commented Aug 19, 2022

NunchakusLei commented Aug 19, 2022

timkpaine commented Aug 19, 2022

samuelcolvin commented Aug 22, 2022

efisoft-elt commented Feb 24, 2023 • edited

timkpaine commented May 19, 2022 •

edited by samuelcolvin

AngellusMortis commented Aug 8, 2022 •

edited

AngellusMortis commented Aug 8, 2022 •

edited

timkpaine commented Aug 8, 2022 •

edited

timkpaine commented Aug 19, 2022 •

edited

NunchakusLei commented Aug 19, 2022 •

edited

efisoft-elt commented Feb 24, 2023 •

edited