-
Notifications
You must be signed in to change notification settings - Fork 3.6k
More Precision for Float to String Casts #4479
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
Conversation
Fix PHPOffice#3899. Supersedes PR PHPOffice#4476, which will be changed to draft status and closed if this PR is merged. A standard cast from float to string in PHP can drop trailing decimal positions. This can lead to problems above and beyond the usual problems associated with floating point. See the superseded PR for a more complete explanation. `StringHelper::convertToString` is changed for how it handles floats. It will now do separate casts for the whole and decimal parts, and then combine the results. This affects `Cell::getValueString` and `Cell::getCalculatedValueString`. Xlsx Writer will now invoke `convertToString` before writing a float to Xml. Ods Writer already uses `getValueString`, so no change is needed there. Xls Writer writes its float values in binary, so no change is needed there. Tests are added for all 3 writers. Aside from fixing some problems, it might appear that this change introduces some new problems. For instance, setting a cell to `12345.6789` will now result in `12345.67890000000079` in the Xml. This difference is an illusion, merely a consequence of floating point rounding. If you run the following check under PhpUnit, it will pass: ```php self::assertSame(12345.6789, 12345.67890000000079); ```
Scrutinizer "error" is, once again, a false positive caused by its inability to properly interpret |
@oleibman This should probably be documented somewhere, it was a breaking change for me. I was using |
@hackel The release notes for 4.3.0 include the following: More Precision for Float to String Casts. Issue #3899 PR #4479 So your statement that it wasn't mentioned at all is not so. If you think there is a better way to have mentioned it, please suggest how and I will try to apply your suggestion to future release notes. I am sorry that it was a breaking change for you. I do not understand why Php truncates its float to string casts, but its doing so was responsible for several problems which this change fixed. And, as I stated above, differences due to this change are a bit of an illusion (although apparently not so in your case). My apologies for the inconvenience. |
@oleibman You're right, I'm sorry I was looking at the release notes for 4.3.1. I was thinking that it might necessitate a major version bump, but I can see now how it's considered just a bugfix. Luckily the solution wasn't too difficult for me. Thanks for your work on this, it is appreciated very much! |
@blizzz Sorry this is still a problem for you. If you have a specific code/spreadsheet combination that is not working for you, I will take another look. |
@blizzz I will carry on this discussion in your PR rather than continuing to add to this closed one. |
Fix #3899. Supersedes PR #4476, which will be changed to draft status and closed if this PR is merged.
A standard cast from float to string in PHP can drop trailing decimal positions. This can lead to problems above and beyond the usual problems associated with floating point. See the superseded PR for a more complete explanation.
StringHelper::convertToString
is changed for how it handles floats. It will now do separate casts for the whole and decimal parts, and then combine the results. This affectsCell::getValueString
andCell::getCalculatedValueString
. Xlsx Writer will now invokeconvertToString
before writing a float to Xml. Ods Writer already usesgetValueString
, so no change is needed there. Xls Writer writes its float values in binary, so no change is needed there. Tests are added for all 3 writers.Aside from fixing some problems, it might appear that this change introduces some new problems. For instance, setting a cell to
12345.6789
will now result in12345.67890000000079
in the Xml. This difference is an illusion, merely a consequence of floating point rounding. If you run the following check under PhpUnit, it will pass:This is:
Checklist: