feat: avoid reconstructing models used as fields of another model on validation #2193
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2193 +/- ##
==========================================
+ Coverage 99.88% 99.93% +0.04%
==========================================
Files 22 22
Lines 4351 4351
Branches 875 875
==========================================
+ Hits 4346 4348 +2
+ Misses 5 3 -2
Continue to review full report at Codecov.
|
…validation closes pydantic#265 closes pydantic#1837
PrettyWood
force-pushed
the
feat/inherit-model-avoid-reconstruction
branch
from
4da511e
to
e189506
Compare
changes/265-PrettyWood.md
Outdated
| @@ -0,0 +1,3 @@ | |||
| **Possible Breaking Change:** Inherited models as fields are not reconstructed (copied) anymore on validation | |||
| To keep old behaviour, set `copy_on_model_validation = True` in the `Config` of your `BaseModel`. | |||
| To change it globally for all your _pydantic_ models, you can set `BaseConfig.copy_on_model_validation = True` | |||
There was a problem hiding this comment.
probably best to move some of this to the docs.
There was a problem hiding this comment.
Added the section from the other PR to explain how to change the behaviour globally.
Added the feature flag in the doc and change the default value to avoid any breaking change.
Ready for another review :)
|
Hi @PrettyWood unfortunately, merging this has caused tests to fail: https://github.com/samuelcolvin/pydantic/runs/1887580866 Any chance you could look into it? When I get a chance, I'll move these binary builds to be run on PR tests as well as master to avoid problems like this. There's no way you could have predicted this, in fact I have no idea why those tests are any different to earlier tests with pydantic compiled. |
|
Damn sorry...on it! |
Change Summary
Right now we always copy a
BaseModelwhen validated, which should not be the default behaviour IMHO.While working on this I thought about "feature flags" to be able to change the behaviour of all pydantic models but probably other classes / metaclasses Don't know if we want to put everything in the
BaseConfigor if we want to separate what's global and local to aBaseModel.ℹ️ I opened a discussion #2194 and added some documentation if we stick to the current way in #2211
Depending on what's decided, I'll update this PR and add some documentation
Related issue number
closes #265
closes #1246
closes #1837
Checklist
changes/<pull request or issue id>-<github username>.mdfile added describing change(see changes/README.md for details)