fix(csvstat): --decimal-format strips significant zeroes, and comma locales get a dangling separator

[VXNCXNX]

What's broken

format_decimal calls .rstrip('0').rstrip('.') unconditionally, which is only safe when the formatted string actually contains a decimal point.

A zero-decimal format loses significant digits.

$ printf 'n\n120\n' > z.csv
$ csvstat -c n --max --decimal-format '%.0f' z.csv
12

There is no decimal point in 120, so the rstrip('0') eats a real digit. %d and %.0e are affected the same way.

Any comma-decimal locale gets a dangling separator on every whole number.

$ LC_ALL=de_DE.UTF-8 csvstat --csv n.csv
1,n,Number,False,2,2,"1.000,","2.000,","3.000,","1.500,","1.500,","707,107",,0,"1000, 2000"

$ LC_ALL=C csvstat --csv n.csv
1,n,Number,False,2,2,1000,2000,3000,1500,1500,707.107,,0,"1000, 2000"

1.000,000 becomes 1.000,. The rstrip('0') removes the fractional zeros, then rstrip('.') cannot clean up the trailing separator because it only knows about ..

The fix

Strip only when the locale's decimal separator is actually present, and strip that separator rather than a hardcoded .:

decimal_point = locale.localeconv()['decimal_point']
if decimal_point and decimal_point in formatted:
    formatted = formatted.rstrip('0').rstrip(decimal_point)

After:

%.0f over 120        -> 120
default over 1.5     -> 1.5        (still trimmed)
de_DE 1000           -> 1.000      (was 1.000,)
de_DE stddev         -> 707,107    (unchanged)
%d over 120          -> 120
%.0e over 120        -> 1e+02

This is already visible in your own test suite

Under a comma-decimal locale, tests/test_utilities/test_csvstat.py fails 3 of 23 on master, for exactly this reason. With the fix it is 23 of 23. The full suite goes from 350 passed and 4 failed to 350 passed and 1 failed under de_DE.UTF-8, and the one that remains is test_csvlook.py::test_no_number_ellipsis, which is a different utility and fails identically on an untouched tree.

Under en_US.UTF-8 the suite is 351 passed before and after, so nothing regressed for the common case.

A note on the tests

csvstat.py calls locale.setlocale(locale.LC_ALL, '') at import, so the existing test_decimal_format assertions were environment-dependent while spelling their separators literally as , and .. They now derive both from locale.localeconv(), which is what made them pass under en_US and fail under de_DE while testing the same thing.

I did not touch that import-time setlocale, since it is a separate concern.

Verification

A --decimal-format '%.0f' case added to test_decimal_format, which had three assertions but none with a zero-decimal format.

With the fix reverted and the tests kept: 1 failure under en_US, 3 under de_DE.

Changelog entry added under Unreleased.

[VXNCXNX]

Both of these have been open a month behind a red X whose logs have since expired, so here is a fresh check.

Both branches are level with master and pass the actual lint gate and the full suite:

flake8 .            0
isort . --check     0
pytest              351 passed, 4 skipped   (#1353)

One thing worth flagging, because it bit me while checking: test_decimal_format on master hardcodes 9,748.346, but csvstat calls locale.setlocale(locale.LC_ALL, '') on import, so the separators follow the environment. That test fails on master under LC_ALL=C (9748.346) and under a comma locale (9.748,346). #1353 makes those assertions read locale.localeconv() instead, which is why it passes everywhere. That is incidental to the fix, not the point of it, but it does mean the test stops depending on who runs it.

• fix(csvstat): --decimal-format strips significant zeroes, and comma locales get a dangling separator #1353, +22/-4: --decimal-format '%.0f' strips significant zeroes, so a minimum of 60 prints as 6. Comma locales also get a dangling separator.
• fix(cli): --zero breaks open-ended column ranges #1354, +16/-2: --zero breaks open-ended column ranges, so -c 3- selects nothing.

Happy to rebase either.