Perform validation on assignment to attribute #94

petroswork · 2017-10-25T05:37:05Z

Add config variable "validate_assignment" defaulting to False.
Add unit test.
Validate on assignment to attribute #92

* Add config variable "validate_assignment" defaulting to False. * Add unit test.

codecov · 2017-10-25T05:37:08Z

Codecov Report

Merging #94 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master    #94   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          11     11           
  Lines         821    827    +6     
  Branches      182    184    +2     
=====================================
+ Hits          821    827    +6

petroswork · 2017-10-25T05:42:13Z

I'm not certain what the second parameter of validate is for, so I left it empty.

samuelcolvin

Thanks so much for your contribution, but a few things to fix here.

By the way, you don't need to delete your pull request and create another one to make an update. Instead just push more commits to the branch which you used for this pull request (in this case your master).

samuelcolvin · 2017-10-25T10:31:28Z

pydantic/main.py

@@ -111,7 +112,13 @@ def __setattr__(self, name, value):
            raise ValueError(f'"{self.__class__.__name__}" object has no field "{name}"')
        elif not self.config.allow_mutation:
            raise TypeError(f'"{self.__class__.__name__}" is immutable and does not support item assignment')
-        self.__values__[name] = value
+        if self.config.validate_assignment:


samuelcolvin · 2017-10-25T10:33:11Z

pydantic/main.py

+            value_, error_ = self.fields[name].validate(value, {})
+            if error_:
+                raise ValidationError({name: error_})
+            self.__values__[name] = value_


please use

if error_: raise ValidationError({name: error_}) else: self.__values__[name] = value_

I know the else is not required but I think it makes the code more readable.

samuelcolvin · 2017-10-25T10:33:41Z

tests/test_main.py

+
+
+def test_validating_assignment():
+    import pydantic


you don't need this import

So add from pydantic import constr to the top of the file.

samuelcolvin · 2017-10-25T10:33:53Z

tests/test_main.py

+def test_validating_assignment():
+    import pydantic
+
+    class Pedantic(BaseModel):


use ValidateAssignmentModel as the name here and move outside the test so it can be used in multiple tests.

samuelcolvin · 2017-10-25T10:35:34Z

tests/test_main.py

+    p.b = "hi"
+    assert p.b == "hi"
+    assert p.values() == {"a": 2, "b": "hi"}
+    with pytest.raises(ValidationError) as exc_info:


split the failure into a separate test.

samuelcolvin · 2017-10-25T10:36:56Z

tests/test_main.py

+        class Config:
+            validate_assignment = True
+
+    p = Pedantic(a=5, b="hello")


use single quotes for strings unless they include single quotes.

samuelcolvin · 2017-10-25T10:37:09Z

tests/test_main.py

+        p.b = ""
+    assert "error validating input" in str(exc_info)
+
+    class Permissive(BaseModel):


move all this into separate tests.

samuelcolvin · 2017-10-25T10:40:17Z

pydantic/main.py

@@ -111,7 +112,13 @@ def __setattr__(self, name, value):
            raise ValueError(f'"{self.__class__.__name__}" object has no field "{name}"')
        elif not self.config.allow_mutation:
            raise TypeError(f'"{self.__class__.__name__}" is immutable and does not support item assignment')
-        self.__values__[name] = value
+        if self.config.validate_assignment:
+            value_, error_ = self.fields[name].validate(value, {})


the second argument to validate() should be the other values on the modal in case validation of this field needs to see these values. Here I guess you should use self.values(exclude={name}).

You should also add a test for this case.

* Improved tests per maintainer's suggestions.

petroswork · 2017-10-26T06:24:51Z

The second parameter to fields.validate does not seem to work, at least not in the way I have in mind, hence the commented out test. Any suggestions?

samuelcolvin

Nearly there!

We also need to add a section to docs and a line to HISTORY.rst. If that's too boring, I'll do it once this is ready and we've merged it.

samuelcolvin · 2017-10-26T09:36:14Z

tests/test_main.py

+class ValidateAssignmentModel(BaseModel):
+    a: int = 2
+    b: constr(min_length=1)
+    c: constr(min_length=a)


This doesn't make sense. Remove c completely.

As per #29 I need to improve how validators work, so I guess it's not really possible to test this case. Sorry.

I mean #15 not #29.

samuelcolvin · 2017-10-26T09:39:16Z

tests/test_main.py

+        validate_assignment = True
+
+
+def test_validating_assignment():


Unit tests are supposed to test one thing, so if the test fails you know what part of the code has gone wrong.

This isn't always simple, but we can do better here. Split this into test_validating_assignment_pass and test_validating_assignment_fail.

samuelcolvin · 2017-10-26T09:42:13Z

tests/test_main.py

+    assert p.values() == {'a': 2, 'b': 'hi', 'c': 'testing'}
+    with pytest.raises(ValidationError) as exc_info:
+        p.a = 'b'
+    assert 'error validating input' in str(exc_info)


almost all ValidationErrors will include 'error validating input' either remove this or check the inner error is right.

samuelcolvin · 2017-10-26T09:42:33Z

tests/test_main.py

+    with pytest.raises(ValidationError) as exc_info:
+        p.c = 'c'
+    assert 'error validating input' in str(exc_info)
+    p.a = 100


I guess remove from here on, I'll add a test once I improve validators.

samuelcolvin · 2017-10-31T20:04:44Z

Awesome, thank you.

Perform validation on assignment to attribute

abc9480

* Add config variable "validate_assignment" defaulting to False. * Add unit test.

petroswork changed the title ~~Perform validation on assignment to attribute~~ Perform validation on assignment to attribute #92 Oct 25, 2017

petroswork changed the title ~~Perform validation on assignment to attribute #92~~ Perform validation on assignment to attribute Oct 25, 2017

samuelcolvin requested changes Oct 25, 2017

View reviewed changes

Add non-empty second parameter to fields.validate

7bb90ad

* Improved tests per maintainer's suggestions.

samuelcolvin requested changes Oct 26, 2017

View reviewed changes

petroswork added 2 commits October 28, 2017 18:49

Improved tests.

cc84120

Add documentation in index.rst

caa878d

samuelcolvin merged commit fe80317 into pydantic:master Oct 31, 2017

This was referenced Oct 31, 2017

Validate on assignment to attribute #92

Closed

Better validators #97

Merged

cdeil mentioned this pull request Nov 19, 2019

Data validation on attribute setting? s-knibbs/dataclasses-jsonschema#117

Closed

gsakkis mentioned this pull request Mar 1, 2023

can't get previous value of the field inside validator #4888

Closed

15 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Perform validation on assignment to attribute #94

Perform validation on assignment to attribute #94

petroswork commented Oct 25, 2017 •

edited

Loading

codecov bot commented Oct 25, 2017 •

edited

Loading

petroswork commented Oct 25, 2017 •

edited

Loading

samuelcolvin left a comment

samuelcolvin Oct 25, 2017

samuelcolvin Oct 25, 2017

samuelcolvin Oct 25, 2017

samuelcolvin Oct 25, 2017

samuelcolvin Oct 25, 2017

samuelcolvin Oct 25, 2017

samuelcolvin Oct 25, 2017

samuelcolvin Oct 25, 2017

samuelcolvin Oct 25, 2017

petroswork commented Oct 26, 2017

samuelcolvin left a comment

samuelcolvin Oct 26, 2017

samuelcolvin Oct 26, 2017

samuelcolvin Oct 26, 2017

samuelcolvin Oct 26, 2017

samuelcolvin Oct 26, 2017

samuelcolvin commented Oct 31, 2017

Perform validation on assignment to attribute #94

Perform validation on assignment to attribute #94

Conversation

petroswork commented Oct 25, 2017 • edited Loading

codecov bot commented Oct 25, 2017 • edited Loading

Codecov Report

petroswork commented Oct 25, 2017 • edited Loading

samuelcolvin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petroswork commented Oct 26, 2017

samuelcolvin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samuelcolvin commented Oct 31, 2017

petroswork commented Oct 25, 2017 •

edited

Loading

codecov bot commented Oct 25, 2017 •

edited

Loading

petroswork commented Oct 25, 2017 •

edited

Loading