|Summary:||[PATCH] Some Excel formats are formatted incorrectly by DataFormatter|
|Component:||XSSF||Assignee:||POI Developers List <dev>|
3/4's of a patch, might be good enough
patch for fraction format
Fraction cells in 2003 format
Description abramova151288 2013-03-13 12:30:34 UTC
Created attachment 30044 [details] DataFormatter changes Some Excel formats are displayed incorrect in the application (fractional, accounting and currency formats). For example, this formats displays incorrect: Category - Fraction: 1/4 (Up to one digit (1/4) cell type) --> 0/ 21/25 (Up to two digits (21/25) cell type) --> 1/ 1/2 (As halves (1/2) cell type) --> 0/2 2/4 (As quarters (2/4) cell type) --> 0/4 Category - Accounting: 0,001000000 (Decimal places - 9, Symbol - р.) --> 0,00 $ 1,234567890 (Decimal places - 9, Symbol - $English(Canada)) --> 1,23 Category - Currency: 0,001000000 (Decimal places - 9, Symbol - р.) --> 0,00 $ 1,234567890 (Decimal places - 9, Symbol - $English(Canada)) --> 1,23
Comment 1 Nick Burch 2013-03-13 13:15:30 UTC
Are you able to work up a quick unit test to go with this? Just something that tries to format a couple of different numbers with each of the formats. That way, we'll know it's fixed, and we'll also be able to make sure it doesn't get broken in future!
Comment 2 Tim Allison 2013-06-13 19:45:14 UTC
Created attachment 30430 [details] 3/4's of a patch, might be good enough Thanks to Ryan Krueger and https://issues.apache.org/jira/browse/TIKA-1132 for the custom formatting test case on fractions. I added some test cases, and offer a 3/4's patch. This patch passes 3148 tests and fails on 38. Without the patch, there are 3062 failurs and 124 successes. I refactored the fraction calculator to avoid the n^2 calculations that could lead to 100,000,000 calculations if the format was ##/####. By "refactored," I mean copied and pasted from org.apache.math.fraction.
Comment 3 Nick Burch 2013-06-13 21:31:47 UTC
We debated adding a dependency on commons math, but decided against it. Copying their code is probably more sensible, though someone came up with a contribution which we accepted in the mean time... Hopefully someone with slightly stronger math skills can offer an opinion on your suggested fix!
Comment 4 Tim Allison 2013-06-13 23:53:47 UTC
Created attachment 30431 [details] patch for fraction format Completely agree. Should have stepped away from the computer before submitting the last. This passes all tests. Still only handles fraction formatting.
Comment 5 Nick Burch 2013-06-20 13:38:17 UTC
Thanks for this patch Tim! For the unit test, I renamed it to TestFractionFormat + changed package, and updated how it gets the test text file to be consistent with the xls, as well as re-enabling the single test One thing I've just found - inner class Fraction of CellNumberFormat - seems to do something similar. Any chance you could cast an eye over it, and decide if we can rationalise / refactor to just have the one? Patch applied (with tweaks described above) in r1494986, thanks
Comment 6 Tim Allison 2013-06-20 15:08:50 UTC
(In reply to Nick Burch from comment #5) > Thanks for this patch Tim! > > For the unit test, I renamed it to TestFractionFormat + changed package, and > updated how it gets the test text file to be consistent with the xls, as > well as re-enabling the single test > > One thing I've just found - inner class Fraction of CellNumberFormat - seems > to do something similar. Any chance you could cast an eye over it, and > decide if we can rationalise / refactor to just have the one? > > Patch applied (with tweaks described above) in r1494986, thanks Thank you, Nick. Will do.
Comment 7 Chris Lott 2013-10-28 13:08:37 UTC
Created attachment 30981 [details] Fraction cells in 2003 format Adding an excel sheet suitable for use as test of fraction formatting, HTH
Comment 8 Tim Allison 2013-10-28 18:27:02 UTC
Thank you, Chris. POI-55419 covers the examples you offered. Fraction formatting is still not perfect...there are some rounding differences btwn Java and MSExcel...but I think the fraction component of this issue should be closed. I haven't had a chance to look into the currency and accounting items that are raised by this issue. If you have a need and POI is failing for you in those areas, please submit a file that shows those problems. Thank you, again!
Comment 9 Nick Burch 2014-07-30 16:16:42 UTC
Just in case it isn't clear - we're still waiting on some new tests (unit test + sample file) that shows up the last remaining parts of this issue around currenct / accounting formatting With those, it should be possible to investigate what logic changes are still needed, then work on those.
Comment 10 Nick Burch 2015-10-25 21:50:37 UTC
The currency / accounting pieces may have been solved by the work for #58536, or at least partially, would be great if someone could check and write a unit test for any parts that remain outstanding!