{"action":"opened","number":374,"pull_request":{"url":"https://api.github.com/repos/schematics/schematics/pulls/374","id":53347900,"html_url":"https://github.com/schematics/schematics/pull/374","diff_url":"https://github.com/schematics/schematics/pull/374.diff","patch_url":"https://github.com/schematics/schematics/pull/374.patch","issue_url":"https://api.github.com/repos/schematics/schematics/issues/374","number":374,"state":"open","locked":false,"title":"Validation refactoring + exception redesign","user":{"login":"bintoro","id":7081785,"avatar_url":"https://avatars.githubusercontent.com/u/7081785?v=3","gravatar_id":"","url":"https://api.github.com/users/bintoro","html_url":"https://github.com/bintoro","followers_url":"https://api.github.com/users/bintoro/followers","following_url":"https://api.github.com/users/bintoro/following{/other_user}","gists_url":"https://api.github.com/users/bintoro/gists{/gist_id}","starred_url":"https://api.github.com/users/bintoro/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bintoro/subscriptions","organizations_url":"https://api.github.com/users/bintoro/orgs","repos_url":"https://api.github.com/users/bintoro/repos","events_url":"https://api.github.com/users/bintoro/events{/privacy}","received_events_url":"https://api.github.com/users/bintoro/received_events","type":"User","site_admin":false},"body":"This is a refactoring of the validation/conversion machinery that aims to\r\n* further eliminate redundant code\r\n* make the import recursion loop more generic and reusable\r\n* make the exception flow and error aggregation easier to understand.\r\n\r\nIt introduces some new features made possible by the redesign and a few bugfixes as well.\r\n\r\nThe work builds on the pending [Undefined PR](https://github.com/schematics/schematics/pull/372), meaning that those commits appear here as well. \r\n[Here's a diff](https://github.com/schematics/schematics/compare/330e13ef3bedf19793de040e3b920e4aa24889ca...bintoro:validation-refactor) that excludes them and spans the additive commits only.\r\n\r\nSince the diff is pretty big, I'm going to explain most changes below.\r\n\r\n\r\n\r\n### List of changes\r\n\r\n* Combines the validation and conversion code paths. Recursive validation of nested items and submodels is now provided by `MultiType.convert()` (formerly `to_native()`), obviating the need for parallel, validation-specific logic.\r\n\r\n  * The `validate_items()` methods on `ListType` and `DictType` are removed.\r\n  * The `validate_model()` method on `ModelType` is removed.\r\n\r\n* Enables simultaneous conversion and validation during data import.\r\n\r\n  The statement `FooModel(<raw data>, validate=True)` now runs conversion and validation in one go, collecting both types of errors. If the data is likely to clear at least the conversion step, this is more efficient than sequential processing, which requires traversing the entire dataset twice.\r\n\r\n  Here are some timing results from a simple import+validation testcase:\r\n\r\n                       | Sequential | Single-pass  | Speedup |\r\n  -------------------- | ---------: | -----------: |  -----: |\r\n  Success              | 2.855      | 1.691        |  41%    |\r\n  Fail on validation   | 3.164      | 2.003        |  37%    |\r\n  Fail on conversion   | 1.533      | 2.077        | −35%    |\r\n\r\n* Improves the exception design by implementing a bit more structure (see #369).\r\n  * Clear separation between field-level and compound errors and their representations\r\n  * Support for more detailed error data within the atomic messages\r\n\r\n* Fixes the conversion of `ModelType` fields when the input is already a model instance. Previously, the instance would be returned unchanged even if it contained unconverted fields or submodels.\r\n\r\n  If the process originates from `Model.__init__()`, a new instance is created for each submodel, and the input instance is simply used as the raw data, as would happen with a `dict` input. Otherwise, such as during validation, the existing instance is updated in place to preserve object identity.\r\n\r\n* Makes the valid part of a failing dataset available to application code as `DataError.partial_data`.\r\n\r\n  This information used to exist as `ModelConversionError.partial_data` but was only accessible to functions calling `import_loop` directly. It also wasn't aggregated to include submodels. Both of these issues are fixed, so the entire tree of valid data is now included with the final error raised from `Model.validate()` or similar.\r\n\r\n* Fixes a bug where the first failing field-level validator would prevent the rest of a compound field's validators from running.\r\n\r\n* Fixes a bug(?) that caused model-level validation to be skipped entirely if **any** errors occurred during field-level validation.\r\n\r\n  From now on, only those fields that failed the initial validation are skipped, while others will have their model-level validators run normally. However, if there are any **conversion** errors, no model-level validators are invoked, since validators must be able to rely on the correctness of the datatypes.\r\n\r\n* The `data` dictionary passed to model-level validators is now a (shallow) copy; it no longer points to the actual values that may be later used to update the instance.\r\n\r\n* Removes `MultiType.validate()`. The `BaseType` implementation now works universally.\r\n\r\n* Removes an obsolete validator `_check_for_unknown_fields()` from the `validate` module.\r\n\r\n* Fixes a number of tests that were broken due to unreachable assertions inside `with pytest.raises()` statements.\r\n\r\n* Changes `Context` back to a custom class from `namedtuple`. Turns out strict immutability is highly impractical because sometimes you have to establish a partial or empty context outside of `import_loop` to retain a reference to it. The new `Context` class implements a `__setattr__()` that allows setting each attribute just once, which provides the required degree of immutability.\r\n\r\n* The instant conversion of `ModelType` fields when populated through assignment (as in `model_instance.submodel = {'x': 1}`) is now accomplished through a hook that types themselves may implement. This gets rid of the `isinstance` stuff inside `FieldDescriptor.__set__()` and eliminates the bidirectional module dependency.\r\n\r\n\r\n\r\n### Compatibility\r\n\r\nThis is *mostly* just a reorganization that should not cause any huge trouble in a typical setting.\r\n\r\n* Potential issues are likely to be related to the redesigned exceptions:\r\n\r\n  * The switch from `list` to `dict` as the representation of nested `ListType` errors (#307) is clearly a backward-incompatible change, but it's done for a good reason.\r\n\r\n  * Since `ValidationError` is a field-specific construct, aggregate error classes are no longer its subclasses. This will not work:\r\n\r\n    ```python\r\n    try:\r\n        model_instance.validate()\r\n    except ValidationError:\r\n        ...\r\n    ```\r\n\r\n    Using `ModelValidationError` here *will* continue to work, as it's declared a synonym of `DataError`, as is `ModelConversionError`.\r\n\r\n* If a derivative of `ListType` or `DictType` has overridden `validate_items()`, it would now cause validation to occur twice, since the original types no longer need this method. The same applies to `ModelType` and `validate_model()`.\r\n\r\n* If a `Model` subclass has overridden the `validate()` method in the anticipation that it will be called for every submodel throughout a data tree, it will not work as expected. `Model.validate()` only initiates the validation process and is no longer invoked for submodels.\r\n\r\n* Previously, the `data` dictionary passed to model-level validators would be guaranteed to contain entries for all fields because\r\n  * the validators weren't called at all unless field-level validation was a complete success\r\n  * `Undefined` objects would substitute for missing values, which was basically a bug from #372.\r\n\r\n  As both of these issues have now been remedied, validators wanting to look at other fields on the model will have to take care not to blindly access keys that might not exist.\r\n","created_at":"2015-12-11T02:50:17Z","updated_at":"2015-12-11T02:50:17Z","closed_at":null,"merged_at":null,"merge_commit_sha":null,"assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/schematics/schematics/pulls/374/commits","review_comments_url":"https://api.github.com/repos/schematics/schematics/pulls/374/comments","review_comment_url":"https://api.github.com/repos/schematics/schematics/pulls/comments{/number}","comments_url":"https://api.github.com/repos/schematics/schematics/issues/374/comments","statuses_url":"https://api.github.com/repos/schematics/schematics/statuses/bf572b33fccf4ba9c5d883f94343cb052be79b43","head":{"label":"bintoro:validation-refactor","ref":"validation-refactor","sha":"bf572b33fccf4ba9c5d883f94343cb052be79b43","user":{"login":"bintoro","id":7081785,"avatar_url":"https://avatars.githubusercontent.com/u/7081785?v=3","gravatar_id":"","url":"https://api.github.com/users/bintoro","html_url":"https://github.com/bintoro","followers_url":"https://api.github.com/users/bintoro/followers","following_url":"https://api.github.com/users/bintoro/following{/other_user}","gists_url":"https://api.github.com/users/bintoro/gists{/gist_id}","starred_url":"https://api.github.com/users/bintoro/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bintoro/subscriptions","organizations_url":"https://api.github.com/users/bintoro/orgs","repos_url":"https://api.github.com/users/bintoro/repos","events_url":"https://api.github.com/users/bintoro/events{/privacy}","received_events_url":"https://api.github.com/users/bintoro/received_events","type":"User","site_admin":false},"repo":{"id":20411714,"name":"schematics","full_name":"bintoro/schematics","owner":{"login":"bintoro","id":7081785,"avatar_url":"https://avatars.githubusercontent.com/u/7081785?v=3","gravatar_id":"","url":"https://api.github.com/users/bintoro","html_url":"https://github.com/bintoro","followers_url":"https://api.github.com/users/bintoro/followers","following_url":"https://api.github.com/users/bintoro/following{/other_user}","gists_url":"https://api.github.com/users/bintoro/gists{/gist_id}","starred_url":"https://api.github.com/users/bintoro/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bintoro/subscriptions","organizations_url":"https://api.github.com/users/bintoro/orgs","repos_url":"https://api.github.com/users/bintoro/repos","events_url":"https://api.github.com/users/bintoro/events{/privacy}","received_events_url":"https://api.github.com/users/bintoro/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/bintoro/schematics","description":"Python Data Structures for Humans™.","fork":true,"url":"https://api.github.com/repos/bintoro/schematics","forks_url":"https://api.github.com/repos/bintoro/schematics/forks","keys_url":"https://api.github.com/repos/bintoro/schematics/keys{/key_id}","collaborators_url":"https://api.github.com/repos/bintoro/schematics/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/bintoro/schematics/teams","hooks_url":"https://api.github.com/repos/bintoro/schematics/hooks","issue_events_url":"https://api.github.com/repos/bintoro/schematics/issues/events{/number}","events_url":"https://api.github.com/repos/bintoro/schematics/events","assignees_url":"https://api.github.com/repos/bintoro/schematics/assignees{/user}","branches_url":"https://api.github.com/repos/bintoro/schematics/branches{/branch}","tags_url":"https://api.github.com/repos/bintoro/schematics/tags","blobs_url":"https://api.github.com/repos/bintoro/schematics/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/bintoro/schematics/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/bintoro/schematics/git/refs{/sha}","trees_url":"https://api.github.com/repos/bintoro/schematics/git/trees{/sha}","statuses_url":"https://api.github.com/repos/bintoro/schematics/statuses/{sha}","languages_url":"https://api.github.com/repos/bintoro/schematics/languages","stargazers_url":"https://api.github.com/repos/bintoro/schematics/stargazers","contributors_url":"https://api.github.com/repos/bintoro/schematics/contributors","subscribers_url":"https://api.github.com/repos/bintoro/schematics/subscribers","subscription_url":"https://api.github.com/repos/bintoro/schematics/subscription","commits_url":"https://api.github.com/repos/bintoro/schematics/commits{/sha}","git_commits_url":"https://api.github.com/repos/bintoro/schematics/git/commits{/sha}","comments_url":"https://api.github.com/repos/bintoro/schematics/comments{/number}","issue_comment_url":"https://api.github.com/repos/bintoro/schematics/issues/comments{/number}","contents_url":"https://api.github.com/repos/bintoro/schematics/contents/{+path}","compare_url":"https://api.github.com/repos/bintoro/schematics/compare/{base}...{head}","merges_url":"https://api.github.com/repos/bintoro/schematics/merges","archive_url":"https://api.github.com/repos/bintoro/schematics/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/bintoro/schematics/downloads","issues_url":"https://api.github.com/repos/bintoro/schematics/issues{/number}","pulls_url":"https://api.github.com/repos/bintoro/schematics/pulls{/number}","milestones_url":"https://api.github.com/repos/bintoro/schematics/milestones{/number}","notifications_url":"https://api.github.com/repos/bintoro/schematics/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/bintoro/schematics/labels{/name}","releases_url":"https://api.github.com/repos/bintoro/schematics/releases{/id}","created_at":"2014-06-02T16:50:32Z","updated_at":"2014-12-18T22:16:08Z","pushed_at":"2015-12-11T01:02:04Z","git_url":"git://github.com/bintoro/schematics.git","ssh_url":"git@github.com:bintoro/schematics.git","clone_url":"https://github.com/bintoro/schematics.git","svn_url":"https://github.com/bintoro/schematics","homepage":"http://schematics.readthedocs.org/","size":1726,"stargazers_count":0,"watchers_count":0,"language":"Python","has_issues":false,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"schematics:development","ref":"development","sha":"0a6763fdfc5ce40a26b40a3d99b57233fc13dff6","user":{"login":"schematics","id":7866441,"avatar_url":"https://avatars.githubusercontent.com/u/7866441?v=3","gravatar_id":"","url":"https://api.github.com/users/schematics","html_url":"https://github.com/schematics","followers_url":"https://api.github.com/users/schematics/followers","following_url":"https://api.github.com/users/schematics/following{/other_user}","gists_url":"https://api.github.com/users/schematics/gists{/gist_id}","starred_url":"https://api.github.com/users/schematics/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/schematics/subscriptions","organizations_url":"https://api.github.com/users/schematics/orgs","repos_url":"https://api.github.com/users/schematics/repos","events_url":"https://api.github.com/users/schematics/events{/privacy}","received_events_url":"https://api.github.com/users/schematics/received_events","type":"Organization","site_admin":false},"repo":{"id":1199632,"name":"schematics","full_name":"schematics/schematics","owner":{"login":"schematics","id":7866441,"avatar_url":"https://avatars.githubusercontent.com/u/7866441?v=3","gravatar_id":"","url":"https://api.github.com/users/schematics","html_url":"https://github.com/schematics","followers_url":"https://api.github.com/users/schematics/followers","following_url":"https://api.github.com/users/schematics/following{/other_user}","gists_url":"https://api.github.com/users/schematics/gists{/gist_id}","starred_url":"https://api.github.com/users/schematics/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/schematics/subscriptions","organizations_url":"https://api.github.com/users/schematics/orgs","repos_url":"https://api.github.com/users/schematics/repos","events_url":"https://api.github.com/users/schematics/events{/privacy}","received_events_url":"https://api.github.com/users/schematics/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/schematics/schematics","description":"Python Data Structures for Humans™.","fork":false,"url":"https://api.github.com/repos/schematics/schematics","forks_url":"https://api.github.com/repos/schematics/schematics/forks","keys_url":"https://api.github.com/repos/schematics/schematics/keys{/key_id}","collaborators_url":"https://api.github.com/repos/schematics/schematics/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/schematics/schematics/teams","hooks_url":"https://api.github.com/repos/schematics/schematics/hooks","issue_events_url":"https://api.github.com/repos/schematics/schematics/issues/events{/number}","events_url":"https://api.github.com/repos/schematics/schematics/events","assignees_url":"https://api.github.com/repos/schematics/schematics/assignees{/user}","branches_url":"https://api.github.com/repos/schematics/schematics/branches{/branch}","tags_url":"https://api.github.com/repos/schematics/schematics/tags","blobs_url":"https://api.github.com/repos/schematics/schematics/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/schematics/schematics/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/schematics/schematics/git/refs{/sha}","trees_url":"https://api.github.com/repos/schematics/schematics/git/trees{/sha}","statuses_url":"https://api.github.com/repos/schematics/schematics/statuses/{sha}","languages_url":"https://api.github.com/repos/schematics/schematics/languages","stargazers_url":"https://api.github.com/repos/schematics/schematics/stargazers","contributors_url":"https://api.github.com/repos/schematics/schematics/contributors","subscribers_url":"https://api.github.com/repos/schematics/schematics/subscribers","subscription_url":"https://api.github.com/repos/schematics/schematics/subscription","commits_url":"https://api.github.com/repos/schematics/schematics/commits{/sha}","git_commits_url":"https://api.github.com/repos/schematics/schematics/git/commits{/sha}","comments_url":"https://api.github.com/repos/schematics/schematics/comments{/number}","issue_comment_url":"https://api.github.com/repos/schematics/schematics/issues/comments{/number}","contents_url":"https://api.github.com/repos/schematics/schematics/contents/{+path}","compare_url":"https://api.github.com/repos/schematics/schematics/compare/{base}...{head}","merges_url":"https://api.github.com/repos/schematics/schematics/merges","archive_url":"https://api.github.com/repos/schematics/schematics/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/schematics/schematics/downloads","issues_url":"https://api.github.com/repos/schematics/schematics/issues{/number}","pulls_url":"https://api.github.com/repos/schematics/schematics/pulls{/number}","milestones_url":"https://api.github.com/repos/schematics/schematics/milestones{/number}","notifications_url":"https://api.github.com/repos/schematics/schematics/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/schematics/schematics/labels{/name}","releases_url":"https://api.github.com/repos/schematics/schematics/releases{/id}","created_at":"2010-12-27T02:25:29Z","updated_at":"2015-12-07T22:26:14Z","pushed_at":"2015-12-10T03:57:24Z","git_url":"git://github.com/schematics/schematics.git","ssh_url":"git@github.com:schematics/schematics.git","clone_url":"https://github.com/schematics/schematics.git","svn_url":"https://github.com/schematics/schematics","homepage":"http://schematics.readthedocs.org/","size":1986,"stargazers_count":1460,"watchers_count":1460,"language":"Python","has_issues":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":192,"mirror_url":null,"open_issues_count":40,"forks":192,"open_issues":40,"watchers":1460,"default_branch":"development"}},"_links":{"self":{"href":"https://api.github.com/repos/schematics/schematics/pulls/374"},"html":{"href":"https://github.com/schematics/schematics/pull/374"},"issue":{"href":"https://api.github.com/repos/schematics/schematics/issues/374"},"comments":{"href":"https://api.github.com/repos/schematics/schematics/issues/374/comments"},"review_comments":{"href":"https://api.github.com/repos/schematics/schematics/pulls/374/comments"},"review_comment":{"href":"https://api.github.com/repos/schematics/schematics/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/schematics/schematics/pulls/374/commits"},"statuses":{"href":"https://api.github.com/repos/schematics/schematics/statuses/bf572b33fccf4ba9c5d883f94343cb052be79b43"}},"merged":false,"mergeable":null,"mergeable_state":"unknown","merged_by":null,"comments":0,"review_comments":0,"commits":16,"additions":1681,"deletions":646,"changed_files":26}}