Skip to content

py/formatfloat: Fix formatting of whole-numbered floats. #8878

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

Closed
wants to merge 0 commits into from

Conversation

dpwe
Copy link
Contributor

@dpwe dpwe commented Jul 6, 2022

This is to fix issue #4212

Many bugs occur when printing float values containing integer values. The successive-scaling approach in the original algorithm would quickly make these values depart from whole numbers.

This modification attempts to avoid scaling the original value for numbers in the range 1.0 to 1e10, and instead compares with other floats constructed only by multiplying whole numbers, so guaranteed to remain exact until the mantissa runs out.

I added a test that verifies consistency with CPython for a range of values. In fact, CPython also struggles with some values (see, e.g. print(13 * .001)), but at least we have agreement for the values in the test (on my machine).

Signed-off-by: Dan Ellis dan.ellis@gmail.com

@dpwe dpwe changed the title Fix formatting of whole-numbered floats py/floatformat: Fix formatting of whole-numbered floats. Jul 6, 2022
@dpwe dpwe changed the title py/floatformat: Fix formatting of whole-numbered floats. py/formatfloat: Fix formatting of whole-numbered floats. Jul 6, 2022
@dpwe dpwe force-pushed the master branch 2 times, most recently from 397975e to 0352e3e Compare July 6, 2022 23:49
@dpwe dpwe mentioned this pull request Jul 6, 2022
@dpgeorge dpgeorge added the py-core Relates to py/ directory in source label Jul 7, 2022
@dpgeorge
Copy link
Member

dpgeorge commented Jul 7, 2022

Thanks for the contribution, this looks really good! So far my testing of this PR has shown that it does do a better job than before. One test I used was micropython-ulab's tests suite and it fixed some printing issues there.

@dpwe
Copy link
Contributor Author

dpwe commented Jul 8, 2022

After looking at @goatchurchprime’s earlier proposed fix, I changed mine to use integer math for the whole-number part. I also changed the test to only test whole-numbers (since that is the focus of the fix) and added a couple of specific cases.

@codecov-commenter
Copy link

Codecov Report

Merging #8878 (30764a5) into master (474c47d) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #8878      +/-   ##
==========================================
- Coverage   98.31%   98.29%   -0.03%     
==========================================
  Files         157      157              
  Lines       20349    20375      +26     
==========================================
+ Hits        20006    20027      +21     
- Misses        343      348       +5     
Impacted Files Coverage Δ
py/formatfloat.c 99.49% <100.00%> (+0.07%) ⬆️
py/runtime.c 99.01% <0.00%> (-0.71%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 474c47d...30764a5. Read the comment docs.

@dpgeorge
Copy link
Member

There is something wrong with this branch, it now has 2 copies of the patch. Can you please fix it, ideally rebasing on master?

@dpwe
Copy link
Contributor Author

dpwe commented Jul 12, 2022

I'm clumsy with git and couldn't figure out how to fix this without starting over. Please see #8901 for the cleaned-up version of this change.

tannewt added a commit to tannewt/circuitpython that referenced this pull request Feb 7, 2024
…n-main

Translations update from Hosted Weblate
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
py-core Relates to py/ directory in source
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants