Pydantic: Model copy(update=) not validating

Created on 14 Mar 2019  路  7Comments  路  Source: samuelcolvin/pydantic


Question

Currently calling copy with update= does not validate the updated data and we can end up with odd cases like this:

import pydantic

class ChildModel(BaseModel):
    v: int

class Model(BaseModel):
    c: ChildModel

>>> m = Model(c={"v": 5})
>>> print(m.c)
ChildModel v=5

>>> m2 = m.copy(update={"c": {"v": 3}})
>>> print(m2.c)
{'v': 3}

One can also imagine a case where the updated data is completely incorrect as well. A current workaround would be to do the following:

>>> m3 = m.copy(update={"c": m.c.copy(update={"v": 3})})
>> print(m3.c)
ChildModel v=3

This seems a bit awkward. A possible fix would be to call __init__ rather than construct in the copy constructor if update is not None. We use this feature quite a bit to alter immutable models.

feature request

Most helpful comment

how about update complies with validate_assignment, eg. it's validated only if validate_assignment = True?

All 7 comments

this was by design, copy was supposed to be quick and thereby avoid all the validation logic.

I would say, we'd be better to have another method copy_validate (better name required) which copies and updates the model while validating the update data.

At the risk of getting into another discussion the ones recently raging in python-ideas, we could alternatively use the + operator for this, so something like

m3 = m + {"c": {"v": 3}}
print(m3.c)
ChildModel v=3

But maybe that's completely mad?

Maybe a bit too much overloading without the new dict + dict PEP going through in 3.9 or whatever it ends up being?

I would argue a bit that update would be expected to be slightly slower than straight copy. The name does need to be a shorter and have a more clear syntax than Model(**{**other.dict(), **update}) or it isn't worth it.

how about update complies with validate_assignment, eg. it's validated only if validate_assignment = True?

That would certainly cover a case I would consider a bug otherwise, but it wouldn't quite work for our use case. Happy to make a PR to patch validate_assignment however.

ok, what about adding a validate=None argument, means:

  • None - use validate_assignment to decide whether to validate update
  • True - always validate update
  • False - never validate update

?

Thinking on it a bit validate_assignment should work for us, it just involves setting another flag that we do not usually set and remembering to do so. Do you still want a validate=None flag?

I think best to implement this in our own logic.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

ashpreetbedi picture ashpreetbedi  路  3Comments

sbv-trueenergy picture sbv-trueenergy  路  3Comments

dconathan picture dconathan  路  3Comments

engstrom picture engstrom  路  3Comments

iwoloschin picture iwoloschin  路  3Comments