Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow arbitrary types in model #209



Copy link

@oldPadavan oldPadavan commented Jun 27, 2018

Implements #182

Copy link

@codecov codecov bot commented Jun 27, 2018

Codecov Report

Merging #209 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #209      +/-   ##
+ Coverage   99.84%   99.84%   +<.01%     
  Files          11       11              
  Lines        1285     1300      +15     
  Branches      235      237       +2     
+ Hits         1283     1298      +15     
  Misses          2        2

Copy link

@samuelcolvin samuelcolvin left a comment

Otherwise, I think looks good.

if arbitrary_types_allowed:
return [make_arbitrary_type_validator(type_)]
raise errors.ConfigError(f'no validator found for {type_}')
Copy link

@samuelcolvin samuelcolvin Jun 27, 2018

This should be a RuntimeError, it's not a config problem and will happen at runtime.

with pytest.raises(ValidationError) as exc_info:
assert exc_info.value.errors() == [
Copy link

@Gr1N Gr1N Jun 27, 2018

can you please test your changes with exc_info.value.json()? I don't think that ctx with type is JSON serializable. To my mind we need to use utils.display_as_type when we firing ArbitraryTypeError exception.

@@ -204,3 +204,10 @@ class UUIDVersionError(PydanticValueError):

def __init__(self, *, required_version: int) -> None:

class ArbitraryTypeError(PydanticTypeError):
Copy link

@Gr1N Gr1N Jun 27, 2018

I think for better readability we need to define code = 'type_error.arbitrary_type' or rename class to ArbitraryError

@samuelcolvin samuelcolvin merged commit 73015d2 into samuelcolvin:master Jul 2, 2018
3 checks passed
@Gr1N Gr1N mentioned this pull request Jul 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants