-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Correct path in nested models #437
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #437 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 14 14
Lines 2157 2160 +3
Branches 426 427 +1
=====================================
+ Hits 2157 2160 +3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
otherwise looks good, thank you.
tests/test_error_wrappers.py
Outdated
|
||
def test_nested_error(): | ||
class NestedModel1(BaseModel): | ||
class NestedModel2(BaseModel): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this a bit confusing, no need to nest the class definitions, just
class NestedModel3(BaseModel):
x: str
class NestedModel2(BaseModel):
data2: List[NestedModel3]
class NestedModel1(BaseModel):
data1: List[NestedModel2]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
HISTORY.rst
Outdated
@@ -7,6 +7,7 @@ v0.21.1 (unreleased) | |||
.................... | |||
* add ``IPv{4,6,Any}Network`` and ``IPv{4,6,Any}Interface`` types from ``ipaddress`` stdlib, #333 by @pilosus | |||
* add docs for ``datetime`` types, #386 by @pilosus | |||
* fix path in nested models, by @kataev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add PR number here as per other entries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
np
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also will need resolve conflicts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, already
Well, what next? |
Awesome, thank you very much. I'll try and wait for #428 to deploy, but if that takes a long time I'll deploy this in a few days. Hope that's okay. |
Yep, no problem. |
Hello, i find what first part of path is missed on errors in nested models.
Here the fix and tests.