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

Make JSONField support type annotation and OpanAPI document generation #1763

Merged
merged 16 commits into from
Nov 18, 2024

Conversation

LanceMoe
Copy link
Contributor

@LanceMoe LanceMoe commented Nov 9, 2024

Make JSONField support type annotation and OpanAPI document generation

JSONField adds optional generic support, and supports OpenAPI document generation by specifying field_type as a pydantic BaseModel

Description

JSONField is defined as Any, which does not display properly when using FastAPI to automatically generate OpenAPI documents. #1702
This PR did two things:

  • Manually specifying field_type enabled the document to be generated properly.
  • Python generics allow for better type hints in IDEs (Pylance and Mypy).

Motivation and Context

I have been using FastAPI + tortoise-orm as the preferred solution for all my project.
My project front-end uses TypeScript + React, and uses openapi-typescript to automatically convert the OpenAPI document generated by FastAPI into a type that can be used by TypeScript. However, the JSONField of tortoise-orm does not support type hints, which requires a lot of type cast in my front-end code, so I tried to add type support to JSONField.

How Has This Been Tested?

In my project, it works correctly, so I'm going to submit a PR to see if it helps with tortoise-orm. But I'm not sure if it will pass the existing tests. I'd like to add as many tests as possible, and hope to get help from the project maintainers.
(Now I have added some test cases for this PR.)

Screenshots of my project:
image
image
image

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added the changelog accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@LanceMoe
Copy link
Contributor Author

I built a local test environment and changed some code to ensure that the change was compatible with the previous version.
I also added relevant tests and documentation.

image

@LanceMoe
Copy link
Contributor Author

I fixed some code so that it now should pass mypy check (make lint).

image

@abondar
Copy link
Member

abondar commented Nov 17, 2024

@LanceMoe Hi!

Please check out CI - seems like tests are failing

@LanceMoe
Copy link
Contributor Author

@abondar
Thanks for your reply, I had only tested it in python 3.10 before and it worked fine. I just set up the test and it does fail in python 3.8 and 3.9, the reason is because staticmethod is not considered callable before python 3.10.
Now I fixed the test and it should work in older versions of python now.
Please try running CI again, thanks!

image

but Codacy Static Code Analysis throw a problem, the lambda is unnecessary on python >= 3.10, but is necessary for python <= 3.9

image

So I had to move it outside class

image

@coveralls
Copy link

Pull Request Test Coverage Report for Build 11882530003

Details

  • 44 of 49 (89.8%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.02%) to 89.254%

Changes Missing Coverage Covered Lines Changed/Added Lines %
tortoise/fields/data.py 24 29 82.76%
Totals Coverage Status
Change from base Build 11881509020: -0.02%
Covered Lines: 6224
Relevant Lines: 6857

💛 - Coveralls

@abondar abondar merged commit defd780 into tortoise:develop Nov 18, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants