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

fix: parse float to string with precision to a sufficient level #994

Merged
merged 1 commit into from
Mar 10, 2025

Conversation

judehung
Copy link
Member

@judehung judehung commented Mar 10, 2025

fixes #989

The original implemetation uses fmt.Sprintf("%e", value) to convert float to string, which will be truncated and rounded at the 6th decimal point. For a float value that needs more precision (not arbitrary precision), it's better to use strconv.FormatFloat with prec -1 to achieve the smallest number of digits necessary such that ParseFloat will return f exactly. Moreover, as fmt.Sprintf is for more general purpose and strconv.FormatFloat is specifically for formatting a float, the strconv.FormatFloat has better performance than fmt.Sprintf.

The benchmark test shows the strconv.FormatFloat takes 955.5 ns/op and fmt.Sprintf takes 1961 ns/op.

If your build fails due to your commit message not passing the build checks, please review the guidelines here: https://github.com/edgexfoundry/go-mod-core-contracts/blob/main/.github/Contributing.md

PR Checklist

Please check if your PR fulfills the following requirements:

  • I am not introducing a breaking change (if you are, flag in conventional commit message with BREAKING CHANGE: describing the break)
  • I am not introducing a new dependency (add notes below if you are)
  • I have added unit tests for the new feature or bug fix (if not, why?)
  • I have fully tested (add details below) this the new feature or bug fix (if not, why?)
  • I have opened a PR for the related docs change (if not, why?)

Testing Instructions

I only run make unittest to verify if any unit tests breaks.

New Dependency Instructions (If applicable)

fixes edgexfoundry#989

The original implemetation uses fmt.Sprintf("%e", value) to convert
float to string, which will be truncated and rounded at the 6th decimal
point. For a float value that needs more precision (not arbitrary
precision), it's better to use strconv.FormatFloat with prec -1 to
achieve the smallest number of digits necessary such that ParseFloat
will return f exactly. Moreover, as fmt.Sprintf is for more general
purpose and strconv.FormatFloat is specifically for formatting a float,
the strconv.FormatFloat has better performance than fmt.Sprintf.

The benchmark test shows the strconv.FormatFloat takes 955.5 ns/op and
fmt.Sprintf takes 1961 ns/op.

Signed-off-by: Jude Hung <[email protected]>
@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 53.84615% with 6 lines in your changes missing coverage. Please review.

Project coverage is 42.95%. Comparing base (de36d96) to head (37c58d2).

Files with missing lines Patch % Lines
dtos/reading.go 53.84% 4 Missing and 2 partials ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #994      +/-   ##
==========================================
+ Coverage   42.93%   42.95%   +0.01%     
==========================================
  Files         119      119              
  Lines        7208     7220      +12     
==========================================
+ Hits         3095     3101       +6     
- Misses       3770     3774       +4     
- Partials      343      345       +2     

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

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@judehung judehung marked this pull request as ready for review March 10, 2025 10:00
@judehung judehung requested a review from cloudxxx8 March 10, 2025 10:00
Copy link
Member

@cloudxxx8 cloudxxx8 left a comment

Choose a reason for hiding this comment

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

LGTM

@cloudxxx8 cloudxxx8 merged commit 88a54aa into edgexfoundry:main Mar 10, 2025
3 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.

Precision limited on Float64 values: Issue or Design?
3 participants