tballison commented on PR #1473:
URL: https://github.com/apache/poi/pull/1473#issuecomment-6416088099
Overall, :robot: comments:
```
- Numerator overflow: the catch was also seeing improper fractions with long
denominators, e.g. ?????????/????????? on 2500.123456789 (the numerator passes
Integer.MAX_VALUE while the denominator is ~10^7). SimpleFraction now returns
the previous convergent there, with tests.
- Probe order: probing with the cell's value meant a too-large first value
cached FractionFormat for the whole format string, so later cells lost their
text. It now probes with a fixed 1.25, and there's a reverse-order test.
- Per-value fallback: CellFormatWrapper now falls back to the same
FractionFormat rather than General, so 3e9+0.5 shows as 6000000001/2 (Excel's
value minus the text) rather than 3000000001. Your test's expectation changed
accordingly. I also dropped that log to DEBUG, since it can fire once per cell.
- PI: exact assertions use 3.25 as you suggested. I kept a value-free check
that 9, 10 and 20 digits give the same PI fraction. For your follow-up: POI
rounds to 15 significant digits and stops when the convergent equals that
double; Excel stops at the convergent 5419351/1725033, so it looks like an
error tolerance.
Two things I left out of scope: the hasLiteralDecimalFormatCannotShow probe
has the same first-value issue, and direct CellFormat.apply callers now get an
IllegalArgumentException for huge improper fractions that Excel can show (a
long numerator would fix that).
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
