You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@poi.apache.org by GitBox <gi...@apache.org> on 2022/04/07 10:53:17 UTC
[GitHub] [poi] colinwjd opened a new pull request, #321: Fix rounding problems
colinwjd opened a new pull request, #321:
URL: https://github.com/apache/poi/pull/321
Use BigDecimal in some cases in DataFormatter.formatCellValue, to avoid rounding problems.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[GitHub] [poi] pjfanning commented on pull request #321: Fix rounding problems
Posted by GitBox <gi...@apache.org>.
pjfanning commented on PR #321:
URL: https://github.com/apache/poi/pull/321#issuecomment-1094127691
@colinwjd it could be a while until the next POI release. You can build your own version of POI, of course. One workaround that you could try is using the ROUND function, eg ROUND(2.05, 1). The current POI 5.2.2 code seems to correctly round this to 2.1.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[GitHub] [poi] pjfanning commented on a diff in pull request #321: Fix rounding problems
Posted by GitBox <gi...@apache.org>.
pjfanning commented on code in PR #321:
URL: https://github.com/apache/poi/pull/321#discussion_r846096448
##########
poi/src/main/java/org/apache/poi/ss/usermodel/DataFormatter.java:
##########
@@ -950,7 +950,7 @@ private String getFormattedNumberString(Cell cell, ConditionalFormattingEvaluato
if (numberFormat == null) {
return String.valueOf(d);
}
- String formatted = numberFormat.format(d);
+ String formatted = numberFormat.format(new BigDecimal(String.valueOf(d)));
Review Comment:
https://github.com/apache/poi/commit/05e8a165430afd225e95a6db2dd793a959a46a10 - this test case I just added seems to behave as expected without the change you are suggesting
POI is 20 year old code and if DataFormatter has a serious bug like the one you suggest it has then I'm surprised noone else has seen the problem (including me).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[GitHub] [poi] pjfanning commented on pull request #321: Fix rounding problems
Posted by GitBox <gi...@apache.org>.
pjfanning commented on PR #321:
URL: https://github.com/apache/poi/pull/321#issuecomment-1093962178
Thanks - I've applied your change based on the reproducible test case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[GitHub] [poi] pjfanning commented on a diff in pull request #321: Fix rounding problems
Posted by GitBox <gi...@apache.org>.
pjfanning commented on code in PR #321:
URL: https://github.com/apache/poi/pull/321#discussion_r845011704
##########
poi/src/main/java/org/apache/poi/ss/usermodel/DataFormatter.java:
##########
@@ -950,7 +950,7 @@ private String getFormattedNumberString(Cell cell, ConditionalFormattingEvaluato
if (numberFormat == null) {
return String.valueOf(d);
}
- String formatted = numberFormat.format(d);
+ String formatted = numberFormat.format(new BigDecimal(String.valueOf(d)));
Review Comment:
Converting a double to a string and then to a bigdecimal and then to a formatted string - this seems dangerous to me - each conversion introducing more risk of rounding issues.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[GitHub] [poi] pjfanning commented on a diff in pull request #321: Fix rounding problems
Posted by GitBox <gi...@apache.org>.
pjfanning commented on code in PR #321:
URL: https://github.com/apache/poi/pull/321#discussion_r847293692
##########
poi/src/main/java/org/apache/poi/ss/usermodel/DataFormatter.java:
##########
@@ -950,7 +950,7 @@ private String getFormattedNumberString(Cell cell, ConditionalFormattingEvaluato
if (numberFormat == null) {
return String.valueOf(d);
}
- String formatted = numberFormat.format(d);
+ String formatted = numberFormat.format(new BigDecimal(String.valueOf(d)));
Review Comment:
String formatted = numberFormat.format(new BigDecimal(NumberToTextConverter.toText(d)));
This was the first attempt I made - it fixes this github321 case but breaks some existing tests.
That NumberToTextConverter seems to have some logic mimics how Excel takes the 2.0499999999999998 and converts it to 2.05 . If we can find out why the other tests break that this solution might be better.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[GitHub] [poi] xzel23 commented on a diff in pull request #321: Fix rounding problems
Posted by GitBox <gi...@apache.org>.
xzel23 commented on code in PR #321:
URL: https://github.com/apache/poi/pull/321#discussion_r846661566
##########
poi/src/main/java/org/apache/poi/ss/usermodel/DataFormatter.java:
##########
@@ -950,7 +950,7 @@ private String getFormattedNumberString(Cell cell, ConditionalFormattingEvaluato
if (numberFormat == null) {
return String.valueOf(d);
}
- String formatted = numberFormat.format(d);
+ String formatted = numberFormat.format(new BigDecimal(String.valueOf(d)));
Review Comment:
Even if there was a problem with rounding, converting to BigDecimal would be the wrong approach. 2.05 is simply not repräsentablerem as a double, so converting 2.05f to BigDecimal can not suddenly increase accuracy. 2.05f is either > 2.05 or < 2.05 (the real number) - I cannot check since I am traveling without my laptop.
You can submit s bug for this if POI formatting differs from Excel, but I'd reject this fix. If something is wrong, the double rounding should be fixed instead - converting to BigDecimal would introduce a huge performance penalty.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[GitHub] [poi] pjfanning commented on a diff in pull request #321: Fix rounding problems
Posted by GitBox <gi...@apache.org>.
pjfanning commented on code in PR #321:
URL: https://github.com/apache/poi/pull/321#discussion_r846098950
##########
poi/src/main/java/org/apache/poi/ss/usermodel/DataFormatter.java:
##########
@@ -950,7 +950,7 @@ private String getFormattedNumberString(Cell cell, ConditionalFormattingEvaluato
if (numberFormat == null) {
return String.valueOf(d);
}
- String formatted = numberFormat.format(d);
+ String formatted = numberFormat.format(new BigDecimal(String.valueOf(d)));
Review Comment:
and the code in getFormattedNumberString does not use BigDecimal at the moment, so your API note is off topic - prove there is an issue and we will fix it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[GitHub] [poi] colinwjd commented on a diff in pull request #321: Fix rounding problems
Posted by GitBox <gi...@apache.org>.
colinwjd commented on code in PR #321:
URL: https://github.com/apache/poi/pull/321#discussion_r845701701
##########
poi/src/main/java/org/apache/poi/ss/usermodel/DataFormatter.java:
##########
@@ -950,7 +950,7 @@ private String getFormattedNumberString(Cell cell, ConditionalFormattingEvaluato
if (numberFormat == null) {
return String.valueOf(d);
}
- String formatted = numberFormat.format(d);
+ String formatted = numberFormat.format(new BigDecimal(String.valueOf(d)));
Review Comment:
<img width="190" alt="image" src="https://user-images.githubusercontent.com/7942918/162353067-28a23e52-9ff7-425b-93c2-891e2330ed69.png">
<img width="850" alt="image" src="https://user-images.githubusercontent.com/7942918/162353213-43151a9e-d10f-425d-a50b-2e46ed792071.png">
As shown above, the value 2.05 is correctly rounded to 2.1 in excel, but in the parsed result the value is 2.0
The reason is that the Double type has a loss of precision, so we need to use the BigDecimal type.
We need to use the String type to initialize the BigDecimal object, otherwise there will also be precision problems.
It is described in the Java api documentation as follows
> This is generally the preferred way to convert a float or double into a BigDecimal, as it doesn't suffer from the unpredictability of the BigDecimal(double) constructor.
<img width="678" alt="image" src="https://user-images.githubusercontent.com/7942918/162354520-27fbc225-62c9-478b-a065-5cbbc97ad5d4.png">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[GitHub] [poi] pjfanning commented on a diff in pull request #321: Fix rounding problems
Posted by GitBox <gi...@apache.org>.
pjfanning commented on code in PR #321:
URL: https://github.com/apache/poi/pull/321#discussion_r846677479
##########
poi/src/main/java/org/apache/poi/ss/usermodel/DataFormatter.java:
##########
@@ -950,7 +950,7 @@ private String getFormattedNumberString(Cell cell, ConditionalFormattingEvaluato
if (numberFormat == null) {
return String.valueOf(d);
}
- String formatted = numberFormat.format(d);
+ String formatted = numberFormat.format(new BigDecimal(String.valueOf(d)));
Review Comment:
@xzel23 this is merged - it's more important that POI is correct than lightening fast. I have profiled POI and the XSSF is really really slow and the cause is XMLBeans and all the zip and xml parsing and writing. Number parsing barely registers. If you use the latest JVMs, doubles and big-decimal parsing/writing performance has improved considerably. These improvements seem to have been added to the JVMs with little fan fair.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[GitHub] [poi] xzel23 commented on a diff in pull request #321: Fix rounding problems
Posted by GitBox <gi...@apache.org>.
xzel23 commented on code in PR #321:
URL: https://github.com/apache/poi/pull/321#discussion_r847266740
##########
poi/src/main/java/org/apache/poi/ss/usermodel/DataFormatter.java:
##########
@@ -950,7 +950,7 @@ private String getFormattedNumberString(Cell cell, ConditionalFormattingEvaluato
if (numberFormat == null) {
return String.valueOf(d);
}
- String formatted = numberFormat.format(d);
+ String formatted = numberFormat.format(new BigDecimal(String.valueOf(d)));
Review Comment:
This is a terrible workaround and not a fix. 2.05f is less than 2.05, so 2.0 would be the correct result when converting to a BigDecimal. The formatter should be fixed instead. Just check in JShell:
```
axel@xiaolong ~ % jshell
| Welcome to JShell -- Version 18
| For an introduction type: /help intro
jshell> 2.05
$1 ==> 2.05
jshell> new BigDecimal(2.05)
$2 ==> 2.04999999999999982236431605997495353221893310546875
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[GitHub] [poi] asfgit closed pull request #321: Fix rounding problems
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #321: Fix rounding problems
URL: https://github.com/apache/poi/pull/321
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[GitHub] [poi] pjfanning commented on pull request #321: Fix rounding problems
Posted by GitBox <gi...@apache.org>.
pjfanning commented on PR #321:
URL: https://github.com/apache/poi/pull/321#issuecomment-1094037238
I'm still tweaking this code because this change is breaking other tests. The Microsoft developers have come up with such terrible Excel code that it is gut-wrenching trying to hack something that matches their code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[GitHub] [poi] xzel23 commented on a diff in pull request #321: Fix rounding problems
Posted by GitBox <gi...@apache.org>.
xzel23 commented on code in PR #321:
URL: https://github.com/apache/poi/pull/321#discussion_r847300532
##########
poi/src/main/java/org/apache/poi/ss/usermodel/DataFormatter.java:
##########
@@ -950,7 +950,7 @@ private String getFormattedNumberString(Cell cell, ConditionalFormattingEvaluato
if (numberFormat == null) {
return String.valueOf(d);
}
- String formatted = numberFormat.format(d);
+ String formatted = numberFormat.format(new BigDecimal(String.valueOf(d)));
Review Comment:
If I had time... Let's just keep it in like this until there is time to look into the formatter.
BTW just last week I found some hand crafted number formatting code in an arbitrary precision library I once wrote and while I don't remember why I would have implemented such a conversion myself I bet it must have been an issue like this one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[GitHub] [poi] pjfanning commented on a diff in pull request #321: Fix rounding problems
Posted by GitBox <gi...@apache.org>.
pjfanning commented on code in PR #321:
URL: https://github.com/apache/poi/pull/321#discussion_r846083940
##########
poi/src/main/java/org/apache/poi/ss/usermodel/DataFormatter.java:
##########
@@ -950,7 +950,7 @@ private String getFormattedNumberString(Cell cell, ConditionalFormattingEvaluato
if (numberFormat == null) {
return String.valueOf(d);
}
- String formatted = numberFormat.format(d);
+ String formatted = numberFormat.format(new BigDecimal(String.valueOf(d)));
Review Comment:
could you provide the xlsx file? the fact that you are using a formula evaluator suggests that the cells may not have numbers but have formulas instead - this could affect the result
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[GitHub] [poi] xzel23 commented on a diff in pull request #321: Fix rounding problems
Posted by GitBox <gi...@apache.org>.
xzel23 commented on code in PR #321:
URL: https://github.com/apache/poi/pull/321#discussion_r847278871
##########
poi/src/main/java/org/apache/poi/ss/usermodel/DataFormatter.java:
##########
@@ -950,7 +950,7 @@ private String getFormattedNumberString(Cell cell, ConditionalFormattingEvaluato
if (numberFormat == null) {
return String.valueOf(d);
}
- String formatted = numberFormat.format(d);
+ String formatted = numberFormat.format(new BigDecimal(String.valueOf(d)));
Review Comment:
While I can confirm that Excel displays the value as 2.1, this is obviously done in the Excel formatter as the value in the XML is stored as 2.0499999999999998:
```
<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<worksheet xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main" xmlns:r="http://schemas.openxmlformats.org/officeDocument/2006/relationships" xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" mc:Ignorable="x14ac xr xr2 xr3" xmlns:x14ac="http://schemas.microsoft.com/office/spreadsheetml/2009/9/ac" xmlns:xr="http://schemas.microsoft.com/office/spreadsheetml/2014/revision" xmlns:xr2="http://schemas.microsoft.com/office/spreadsheetml/2015/revision2" xmlns:xr3="http://schemas.microsoft.com/office/spreadsheetml/2016/revision3" xr:uid="{7487740C-7451-9943-A628-9BEEC0E38BD2}">
<dimension ref="A1:A6" />
<sheetViews>
<sheetView tabSelected="1" workbookViewId="0">
<selection activeCell="A6" sqref="A6" />
</sheetView>
</sheetViews>
<sheetFormatPr baseColWidth="10" defaultRowHeight="16" x14ac:dyDescent="0.2" />
<sheetData>
<row r="1" spans="1:1" x14ac:dyDescent="0.2">
<c r="A1">
<v>2.04</v>
</c>
</row>
<row r="2" spans="1:1" x14ac:dyDescent="0.2">
<c r="A2" s="1">
<v>2.04</v>
</c>
</row>
<row r="3" spans="1:1" x14ac:dyDescent="0.2">
<c r="A3">
<v>2.0499999999999998</v>
</c>
</row>
<row r="4" spans="1:1" x14ac:dyDescent="0.2">
<c r="A4" s="1">
<v>2.0499999999999998</v>
</c>
</row>
<row r="5" spans="1:1" x14ac:dyDescent="0.2">
<c r="A5">
<v>2.06</v>
</c>
</row>
<row r="6" spans="1:1" x14ac:dyDescent="0.2">
<c r="A6" s="1">
<v>2.06</v>
</c>
</row>
</sheetData>
<pageMargins left="0.7" right="0.7" top="0.75" bottom="0.75" header="0.3" footer="0.3" />
<pageSetup paperSize="9" orientation="portrait" horizontalDpi="0" verticalDpi="0" />
</worksheet>
```
What should be done is check why the BigDecimal formatter displays this *wrong* (or in other words: what kind of workaround is implemented there). It looks like they map everything within one ULP to the rounded value.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[GitHub] [poi] pjfanning commented on a diff in pull request #321: Fix rounding problems
Posted by GitBox <gi...@apache.org>.
pjfanning commented on code in PR #321:
URL: https://github.com/apache/poi/pull/321#discussion_r847293692
##########
poi/src/main/java/org/apache/poi/ss/usermodel/DataFormatter.java:
##########
@@ -950,7 +950,7 @@ private String getFormattedNumberString(Cell cell, ConditionalFormattingEvaluato
if (numberFormat == null) {
return String.valueOf(d);
}
- String formatted = numberFormat.format(d);
+ String formatted = numberFormat.format(new BigDecimal(String.valueOf(d)));
Review Comment:
String formatted = numberFormat.format(new BigDecimal(NumberToTextConverter.toText(d)));
This was the first attempt I made - it fixes this github321 case but breaks some existing tests.
That NumberToTextConverter seems to have some logic that mimics how Excel takes the 2.0499999999999998 and converts it to 2.05 . If we can find out why the other tests break that this solution might be better.
Double.toString also seems to correct for some of these issues to because it too converts 2.0499999999999998 to 2.05.
In the end the madness is why do Microsoft store 2.05 as 2.0499999999999998 in their sheet XML in the first place.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[GitHub] [poi] pjfanning commented on pull request #321: Fix rounding problems
Posted by GitBox <gi...@apache.org>.
pjfanning commented on PR #321:
URL: https://github.com/apache/poi/pull/321#issuecomment-1091602715
@colinwjd could you provide test cases or examples of this issue in practice? Excel is a bit of a mine field when it comes to when to round and when not to round. We have no access to the Excel code or the mindsets of the Microsoft developers. We can only build code based on observation and interpretation of publicly available specs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[GitHub] [poi] xzel23 commented on a diff in pull request #321: Fix rounding problems
Posted by GitBox <gi...@apache.org>.
xzel23 commented on code in PR #321:
URL: https://github.com/apache/poi/pull/321#discussion_r846661566
##########
poi/src/main/java/org/apache/poi/ss/usermodel/DataFormatter.java:
##########
@@ -950,7 +950,7 @@ private String getFormattedNumberString(Cell cell, ConditionalFormattingEvaluato
if (numberFormat == null) {
return String.valueOf(d);
}
- String formatted = numberFormat.format(d);
+ String formatted = numberFormat.format(new BigDecimal(String.valueOf(d)));
Review Comment:
Even if there was a problem with rounding, converting to BigDecimal would be the wrong approach. 2.05 is simply not representable as a double, so converting 2.05f to BigDecimal can not suddenly increase accuracy. 2.05f is either > 2.05 or < 2.05 (the real number) - I cannot check since I am traveling without my laptop.
You can submit s bug for this if POI formatting differs from Excel, but I'd reject this fix. If something is wrong, the double rounding should be fixed instead - converting to BigDecimal would introduce a huge performance penalty.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[GitHub] [poi] colinwjd commented on a diff in pull request #321: Fix rounding problems
Posted by GitBox <gi...@apache.org>.
colinwjd commented on code in PR #321:
URL: https://github.com/apache/poi/pull/321#discussion_r846559707
##########
poi/src/main/java/org/apache/poi/ss/usermodel/DataFormatter.java:
##########
@@ -950,7 +950,7 @@ private String getFormattedNumberString(Cell cell, ConditionalFormattingEvaluato
if (numberFormat == null) {
return String.valueOf(d);
}
- String formatted = numberFormat.format(d);
+ String formatted = numberFormat.format(new BigDecimal(String.valueOf(d)));
Review Comment:
The test file is as follows
[test-github-321.xlsx](https://github.com/apache/poi/files/8456025/test-github-321.xlsx)
The value uses a one-decimal format, in which case the problem I describe occurs.
<img width="898" alt="image" src="https://user-images.githubusercontent.com/7942918/162551906-9ffb13c3-2d27-4146-9a6f-d67ffc150995.png">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[GitHub] [poi] pjfanning commented on a diff in pull request #321: Fix rounding problems
Posted by GitBox <gi...@apache.org>.
pjfanning commented on code in PR #321:
URL: https://github.com/apache/poi/pull/321#discussion_r847273778
##########
poi/src/main/java/org/apache/poi/ss/usermodel/DataFormatter.java:
##########
@@ -950,7 +950,7 @@ private String getFormattedNumberString(Cell cell, ConditionalFormattingEvaluato
if (numberFormat == null) {
return String.valueOf(d);
}
- String formatted = numberFormat.format(d);
+ String formatted = numberFormat.format(new BigDecimal(String.valueOf(d)));
Review Comment:
@xzel23 this is not an argument about 2.05f and how it rounds in Java - it is about having Apache POI match the behaviour of Microsoft Excel. I'm happy to have the solution changed as long the test cases still pass - including the new test case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org