Add support for complex pydantic models #208
Conversation
Codecov Report
@@ Coverage Diff @@
## master #208 +/- ##
=======================================
Coverage 99.91% 99.91%
=======================================
Files 44 44
Lines 2390 2399 +9
Branches 174 179 +5
=======================================
+ Hits 2388 2397 +9
Misses 2 2
Continue to review full report at Codecov.
|
leiserfg
force-pushed
the
complex-pydantic-model
branch
from
1759c7e
to
34fac7a
Compare
|
@prkumar do I need to change something to get this merged? |
There was a problem hiding this comment.
@prkumar do I need to change something to get this merged?
The changes are causing tests to fail in py3.5 and py3.6: https://github.com/prkumar/uplink/pull/208/checks?check_run_id=1298280158. I mention some options of how we may address this in a separate comment:
To solve this, we can either do a try/catch around the code, or we can move this test into a separate file (e.g., test_converters_py37.py) and add some logic to have pytest skip that file when running against versions below 3.7: https://docs.pytest.org/en/latest/example/pythoncollection.html#customizing-test-collection
Other than that, these changes look good to me. However, I have a limited experience with using pydantic. Maybe @gmcrocetti may have some input, as the original contributor of uplink's Pydantic support in #200?
tests/unit/test_converters.py
Outdated
| from typing import List | ||
|
|
||
| class ComplexModel(pydantic.BaseModel): | ||
| when: datetime = datetime.utcnow() |
There was a problem hiding this comment.
These annotations are causing tests to fail with py3.5 and py3.6 due to SyntaxError: https://github.com/prkumar/uplink/pull/208/checks?check_run_id=1298280158
To solve this, we can either do a try/catch around the code, or we can move this test into a separate file (e.g., test_converters_py37.py) and add some logic to have pytest skip that file when running against versions below 3.7: https://docs.pytest.org/en/latest/example/pythoncollection.html#customizing-test-collection
There was a problem hiding this comment.
I changed the test to use comment's annotation so it does not fail on python3.5.
There was a problem hiding this comment.
@prkumar I need this please, I'm tied to the old version of uplink without it.
There was a problem hiding this comment.
@leiserfg - Changes were released yesterday with v0.9.3
leiserfg
force-pushed
the
complex-pydantic-model
branch
from
34fac7a
to
0be222b
Compare
leiserfg
force-pushed
the
complex-pydantic-model
branch
from
0be222b
to
4f8538a
Compare
Fixes #207
Attention: @prkumar