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

✨ Add SimpleJSONResponse for decimal JSON encoding #5023

Closed
wants to merge 5 commits into from
Closed

✨ Add SimpleJSONResponse for decimal JSON encoding #5023

wants to merge 5 commits into from

Conversation

tomy0000000
Copy link

@tomy0000000 tomy0000000 commented Jun 11, 2022

As mentioned in #4998. I believe simplejson is the best option to accomplish high decimal points real number encoding, hence the need of SimpleJSONResponse. I followed some of the parameters used in starlette's original JSONResponse here to make sure the output are the most consistent as possible.

If interested, I'm also willing to draft a page of doc detailing the best practice of handling decimal in FastAPI I've come up so far. Just let me know if that's something you'd like to include.

@codecov
Copy link

codecov bot commented Jun 11, 2022

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (1876ebc) 100.00% compared to head (a837c23) 100.00%.
Report is 1385 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master     #5023   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          532       533    +1     
  Lines        13672     13740   +68     
=========================================
+ Hits         13672     13740   +68     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@github-actions
Copy link
Contributor

📝 Docs preview for commit abf736a at: https://62a4c36223b63b38e9736ac6--fastapi.netlify.app

@github-actions
Copy link
Contributor

📝 Docs preview for commit 1bbee01 at: https://62a4c5ed1ac5a03f5530312d--fastapi.netlify.app

@odiseo0
Copy link

odiseo0 commented Jun 11, 2022

Maybe you can make some tests and write some docs too

@github-actions
Copy link
Contributor

📝 Docs preview for commit 96e3b5f at: https://62ab81ec445265209b184aea--fastapi.netlify.app

@tomy0000000
Copy link
Author

tomy0000000 commented Jun 16, 2022

Hi folks,

as suggested, I've written a page detailing on how to make use of simpleJSONResponse and also some tips on how to make Decimal works with FastAPI.

This is my first time authoring a complete doc for open source project, feedbacks are welcomed 👍🏻

Copy link

@odiseo0 odiseo0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

media_type = "application/json" is not necessary and also you need to write tests

@github-actions
Copy link
Contributor

📝 Docs preview for commit a837c23 at: https://62da846ddace10066081dd5e--fastapi.netlify.app

Copy link

@odiseo0 odiseo0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@theRealSuperMario
Copy link

I could really use this ASAP. Any chance this will get merged soon?

@tomy0000000
Copy link
Author

@tiangolo Would you mind taking a minute to review this? Docs and tests ready, reviewed by others.

@tiangolo tiangolo added feature New feature or request p5 and removed investigate labels Jan 15, 2024
@tiangolo
Copy link
Owner

Thank you @tomy0000000! 🍰

Nevertheless, instead of providing another type of response, I prefer to teach people how to create their own, in particular when creating a new response takes an effort equivalent to learning a new one included, also as not everyone will need it, but if it's included, everything learning could expect to need to learn it. And as it's around 2 or 3 lines of code, I prefer to teach how to use it: https://fastapi.tiangolo.com/advanced/custom-response/#custom-response-class

But you could, for example, write a blog post about using decimal values in JSON responses with your use case, and how to create the SimpleJSONResponse. 🤓

For now, I'll pass on this one, but thanks for the effort! ☕

@tiangolo tiangolo closed this Jan 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request p5
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants