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

Fixes loss when serializing/deserializing large decimals #9180 (take 2) #9292

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ybressler
Copy link
Contributor

@ybressler ybressler commented Apr 19, 2024

Change Summary

Serializing/deserializing large decimal values is lossy. These changes enable expected behavior - no loss in information with large decimals.

Note: This PR replaces work done by #9291

Help Needed

I was able to solve the deserialization part. But I'm having trouble identifying how/where to control for model export to json (serialization is still lossy). <-- that message was on my work in the previous PR. Presently, am not sure where the interface to control serialization/deserialization. Could someone point me in the direction?

Related issue number

fix #9180

Checklist

  • [X ] The pull request title is a good summary of the changes - it will be used in the changelog
  • Unit tests for the changes exist
  • Tests pass on CI
  • Documentation reflects the changes where applicable
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

Copy link

codspeed-hq bot commented Apr 19, 2024

CodSpeed Performance Report

Merging #9292 will not alter performance

Comparing ybressler:ybressler_feat/decoding-decimal-from-json-is-lossy-#9180_ii (b3f0d74) with main (77b0e1c)

Summary

✅ 13 untouched benchmarks

@sydney-runkle
Copy link
Member

Hi @ybressler,

Apologies for the delay here - got distracted with some work on FastUI. My initial thought would be that we'd want to investigate this further in the decimal validator and serializer in pydantic-core. You can find that logic here:

I think @davidhewitt will have the best idea of the issue in core, but feel free to ping me with questions as well!

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.

Decoding Decimal from JSON number is lossy
2 participants