Not sure if this should be classified as a bug or as a feature request.
from typing import Dict, List
from pydantic import BaseModel, ValidationError
class C(BaseModel):
class Config:
validate_assignment = True
f1: List[Dict[int, str]] = []
try:
# for some reason mypy doesn't catch this one
o = C(f1=[1])
print("FAIL")
except ValidationError:
print("PASS")
o = C()
try:
# mypy: List item 0 has incompatible type "int"; expected "Dict[int, str]"
o.f1 = [1]
print("FAIL")
except ValidationError:
print("PASS")
try:
# mypy: Argument 1 to "append" of "list" has incompatible type "int"; expected "Dict[int, str]"
o.f1.append(1)
print("FAIL")
except ValidationError:
print("PASS")
try:
# mypy: Dict entry 0 has incompatible type "str": "int"; expected "int": "str"
o.f1.append({"": 1})
print("FAIL")
except ValidationError:
print("PASS")
o.f1.append({})
try:
# mypy: Incompatible types in assignment (expression has type "int", target has type "str")
o.f1[-1]["x"] = 2
print("FAIL")
except ValidationError:
print("PASS")
results in:
PASS
FAIL
FAIL
FAIL
types <class 'list'> <class 'dict'>
I am well aware of all the philosophical aspects of Python's typing, but in my use case, where user uses Python to gradually build complex configuration model (including incremental adding of items to containers), it would be a much more productive workflow to be able to fail immediately at the location of the offending model modification rather than do a model validation afterwards, making user wonder "hey, where in my hundreds of lines of model I have done this wrong assignment/modification".
It is my understanding, that to be able to achieve this, it would be necessary for the fields of container types to be of custom container types performing all the necessary validations on all operations but from my experience, this is perfectly feasible.
P.S. The mypy has been executed as:
mypy --python-executable venv-pydantic/bin/python --strict pydantic1.py
but this issue is not about mypy.
Please explain what you're asking for?
Are you asking for o.f1.append(1) to raise a validation error?
Exactly
On Tue, 30 Apr 2019, 16:56 Samuel Colvin, notifications@github.com wrote:
Please explain what you're asking for?
Are you asking for o.f1.append(1) to raise a validation error?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/samuelcolvin/pydantic/issues/496#issuecomment-487962148,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAOCW3VOTMN7TVZUHKKHVN3PTBFYNANCNFSM4HJLQL2Q
.
That's not possible I'm afraid. o.f1 is a vanilla list, pydantic doesn't (and can't) know if f1 is mutated.
To do this we would have to implement immutable or at least instrumented version of all mutable types: list, dict, set, orderedDict and everything else. That's beyond the bounds of pydantic unless you know something I don't?
Implementing instrumented versions of the basic container types (list, dict, set, and orderedDict, if you care for Python < 3.6) is the idea. I am unsure as to what you meant by "everything else".
The way I see it, there would be a config option (disabled by default), that would cause all the container properties to be returned as those "instrumented" types. Actually, I would even suggest to design those types to be "accessors" (or "views", if you don't insist that "view" means read-only) to the underlying data. Possibly having raw_data method/property intended as a legitimate escape hatch for accessing the underlying vanilla containers (with, of course, no guarantees for type correctness in case of modifications through them).
Would you be willing to consider this feature if I provide a PR?
I would even suggest to design those types to be "accessors" (or "views", if you don't insist that "view" means read-only) to the underlying data
MutationWatcher would probably be the most sensible name, but I think it would be a lot of work to play nicely with mypy. We're basically talking about building a immutable-js clone for python but with more functionality, eg. validation as well as a hard immutable barrier.
Would you be willing to consider this feature if I provide a PR?
For me this wouldn't be worth the time taken to implement it or the performance impact, but I'll accept a PR if enough other people want it.
I am unsure as to what you meant by "everything else"
Well, almost every object in python is mutable. I could start naming objects that could be mutable and therefore in theory need to be instrumented, but I agree that list, set & dict would take care of a 90% of cases.
I think if you want more bullet proof than that I would suggest you move to rust - that's not a facetious comment, rust is awesome.
but I'll accept a PR if enough other people want it.
How would you measure that? By the number of +1 on this issue?
I plan on implementing this anyway, the question is whether it would be just a part of my tool, or as a part of pydantic.
Well, almost every object in python is mutable.
Yes, but staying with limits of pydantic.BaseModel-derived classes and containers thereof, one can have reasonable run-time type checking.
I think if you want more bullet proof than that I would suggest you move to rust - that's not a facetious comment, rust is awesome.
Well, if I needed a compiled language, I would use C++ just because me and my team are experienced with that.
We're not talking about lots of people, so probably easier via comments with people saying they want it.
If 5 separate people say they're interested in the feature I'll accept a PR.
I think that seems fair?
@kshpytsya any progress on that? Otherwise, I might try to take a shot at this
Humm, I guess I got what I deserved by saying I'd accept it if enough people wanted it. :laughing:
I think the best solution would be to start a new library called immutable-py or similar, then implement __get_validators__ and __modify_schema__ (and perhaps __serialize__(self, protocol) once that's implemented) on each of those types.
immutable-py doesn't even need to have pydantic as a dependency, and I can imagine lots of scenarios where it might be useful outside pydantic. Obviously, you'll want to include docs on using it with pydantic and tests for use with pydantic.
If the library looks well implemented, I'll even run tests against immutable-py in pydantic's test suit as we currently do with fastapi to minimise the chances of breaking changes between versions.
Sorry if this sounds like I'm welshing on my offer above, but I genuinely think this sounds like a better route. I'll happily contribute (both code and ideas) to immutable-py if it gets off the ground, but I already maintain too much open source code, I can't take on another significant chunk of work at this time. Someone else would need to be the driving force.
Does that sound fair?
Unless I misunderstand you want to create a package that supplies immutable containers? I think the original request was to allow mutations and validate them, rather than disallow mutations (like frozenset)
My idea was for pydantic to (optionally) perform validation checks on mutations, something like this:
class PydanticList(list):
def append(self, object):
# do some validations
super(PydanticList, self).append(object)
def __setitem__(self, key, value):
# do some validations
super(PydanticList, self).__setitem__(key, value)
def __iadd__(self, other):
# do some validations
super(PydanticList, self).__iadd__(other)
# ...etc etc
Or, for example, ConstrainedList will override append to check that the max_items parameter is still valid
Obviously this is just the general idea, I didnt really think much of how to implement it, but Id be happy to try.
See https://github.com/MagicStack/immutables for some prior art.
Well, I guess these class could either be immutable or call some validation/check function when being mutated, those methods could then be wired into pydantic.
The more complex questions for me are:
List and list to ImmutableList and immutable_list (or maybe ImList and im_list?)? Or mypy have to consider List and ImList as completely separate types?json.dump, pickle.dump etc? I guess pickle is relatively easy, but json.dump would require a custom default argument.I like the idea in principle, but I think for it to be worth incorporating into pydantic, it should be very decoupled from existing pydantic internals. (For maintainability reasons.)
One idea: add a config setting called validate_mutation that, if True, replaces instances of list with PydanticMutableTypedList or whatever, and similar for other containers.
I am generally opposed to adding config settings but this seems like a pretty fundamental improvement in pydantic's validation capabilities, and may be particularly useful in development, so having a feature-flag-style config setting for this makes sense to me. (Again, it would need to be decoupled -- if it required changes to the internals I don't think it would be worthwhile.)
+1
Most helpful comment
We're not talking about lots of people, so probably easier via comments with people saying they want it.
If 5 separate people say they're interested in the feature I'll accept a PR.
I think that seems fair?