pydantic v2 support #245
pydantic v2 support #245
Conversation
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the GitHub App Integration for your organization. Read more. @@ Coverage Diff @@
## master #245 +/- ##
==========================================
+ Coverage 92.50% 92.72% +0.21%
==========================================
Files 33 33
Lines 2027 2033 +6
==========================================
+ Hits 1875 1885 +10
+ Misses 152 148 -4
|
piccolo_api/fastapi/endpoints.py
Outdated
| for field_name, _field in model.model_fields.items(): | ||
| # get type of field since field.outer_type_ is | ||
| # deprecated in Pydantic V2 (we use that for arrays | ||
| # in the OpenAPI docs). Using vars(), but we can | ||
| # also use _field.annotation.__dict__ (if that's better) | ||
| field_annotation = vars(_field.annotation) | ||
| type_ = field_annotation["__args__"][0] |
There was a problem hiding this comment.
I'm not sure what the best approach is here. I know we used outer_type because if it's list[int] it returns list.
What does _field.annotation give us?
There was a problem hiding this comment.
For array field of strings _field.annotation is typing.Optional[typing.List[str]] but result is wrong
For array field of strings field_annotation["__args__"][0] is typing.List[str] which is correct
I don't know how to solve it otherwise, if you have any other idea feel free to write here.
There was a problem hiding this comment.
Yeah, this was wrecking my brain.
I think I have a solution now - in Python 3.8 they added typing.get_origin and typing.get_args, which means if we have t.Optional[someType] we're able to extract someType.
Your solution probably would have been fine. I was just worried because for field_annotation["__args__"][0] to work we needed it to always be like (someType, None) and not (None, someType).
There was a problem hiding this comment.
I just tried this and the _get_type function doesn't work as it should. The problem remains the same because the Array column does not display correct in the OpenAPI document filters. Now it's like this.
If I change the _get_type function to something like this
def _get_type(type_: t.Type) -> t.Type:
"""
Extract the inner type from an optional if necessary, otherwise return
the type as is.
"""
args = t.get_args(type_)
return args[0]the result is correct and looks like this.
Can you check that in case I'm wrong or doing something wrong.
There was a problem hiding this comment.
You're right - I can see the problem when trying to use the Director.years_nominated filter in the Swagger docs.
Will investigate.
There was a problem hiding this comment.
It's a bit odd, but t.get_args returns this: (typing.List[int], <class 'NoneType'>) and I was expecting (typing.List[int], None).
I need to work out what NoneType is vs None.
There was a problem hiding this comment.
So type(None) is NoneType. I've changed it to check for None and NoneType, and it should work.
Will add some tests.
|
@dantownsend Any updates or new thoughts on this? I think the Piccolo API and Piccolo Admin are equally important parts, as the ORM, in the Piccolo ecosystem and it would be nice if they also have support for Pydantic V2 as soon as possible. Libraries without this support fall out of use, which would be a real shame, because Piccolo is one of the best-maintained, feature-rich libraries out there. |
|
@sinisaos Yeah, I'm running a bit behind of these PRs - has been a busy couple of weeks. But I plan on getting them out asap, because as you say we need them all released, and not just Piccolo. |
* pydantic v2 support (#245) * pydantic_v2_support * pin to Piccolo v1 * add comment * remove KeyError Exception * upgrade coverage * fix type warnings with `nested` * fix mypy warnings * update black * replacement for `outer_type` * change assertion --------- Co-authored-by: Daniel Townsend <[email protected]> * update github actions * bumped version * fix `_get_type` - check for `NoneType` (#254) * fix `_get_type` - check for `NoneType` * add tests * fix typo * add a test for the new union syntax * also check for `UnionType` * bumped version * update JSON schema (#257) * bumped version * Update requirements.txt --------- Co-authored-by: sinisaos <[email protected]>
Related to piccolo-orm/piccolo_admin#313.
The tests will fail until we pin it to the first version of Piccolo with Pydantic V2 support, when it is released.