revalidate models

[samuelcolvin]

fix #155 and fix #21

[codecov]

Codecov Report

Merging #177 (cf04609) into main (cf04609) will not change coverage.
The diff coverage is n/a.

❗ Current head cf04609 differs from pull request most recent head 53d8987. Consider uploading reports for the commit 53d8987 to get more accurate results

@@           Coverage Diff           @@
##             main     #177   +/-   ##
=======================================
  Coverage   97.75%   97.75%           
=======================================
  Files          45       45           
  Lines        4371     4371           
  Branches       32       32           
=======================================
  Hits         4273     4273           
  Misses         98       98           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cf04609...53d8987. Read the comment docs.

[samuelcolvin]

@tiangolo could you confirm you're happy with this behaviour for fastAPI.

Summary of behavior

The validator checks the input value type - effectively type(input_value) == OutputClass, if True

• when revalidate_models=False, the input value is output with no further changes.
• when revalidate_models=True, the input value is completely re-validated using from_attributes, __fields_set__ is copied to the output model if it exists.

There's no isinstance() check anymore.

Note, one potential confusion with this - if you have revalidate_models=True (which I guess will be the default?), and you pass a model instance to the validator, you must have from_attributes=True, otherwise we can't do the re-validation.

Perhaps we make them both true by default. @PrettyWood do you agree?

Perhaps we could:

• change the default for from_attributes on a type-dict validator depending on whether it's the child of a model-class validator
• raise an error if from_attributes!=True on a type-dict when it's the child of a model-class validator with revalidate_models=True

???

[tiangolo]

This looks great! Indeed, it solves the use case in FastAPI. This means I won't have to do the hacky trick to "clone" a field for response_model in FastAPI, so this will be a lot safer and faster. 🎉 🚀

I agree that it makes sense to have both revalidate_models=True and from_attributes=True by default. But I'm of course biased towards what's most useful for FastAPI and SQLModel.

[PrettyWood]

I understand it's better for FastAPI but personally would have kept false by default. I feel like avoiding copies and stuff is the best option in most validation usecases and will allow other libraries to be built on top of pydantic-core. I would have hence just raised an error if both attributes were not set properly.

[samuelcolvin]

I would have hence just raised an error if both attributes were not set properly.

Can do, but we'll need a default in pydantic anyway, we don't want to force people to set config on ever model.

[samuelcolvin]

let's keep this as it is and consider defaults in pydantic, further down the road.