From 3fb9247903619447731121288859bbf4660225eb Mon Sep 17 00:00:00 2001 From: Kevin Costner Date: Thu, 17 Sep 2026 00:30:57 +0530 Subject: [PATCH] fix(semantic): stop assuming a US locale for every currency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit parse_currency deleted every comma and took the dot as the decimal point, whatever currency was present: cleaned = re.sub(r"[A-Za-z$€£¥₹,\s]", "", s) return float(cleaned) An explicit EUR or € is the strongest available signal that the opposite convention is in use, and it was ignored. Measured before this change: EUR 1.200,50 -> 1.2005 (1000x low) EUR 1.000,00 -> 1.0 (1000x low) €0,50 -> 50.0 (100x high: fifty cents became fifty euros) EUR 12,5 -> 125.0 (10x high) €1.234.567,89 -> None (failed to parse) US formats were all correct, so the defect was invisible to a US-shaped test suite. It was reachable from fd.clean: under semantic_mode="auto" the repair landed automatically at confidence 0.98 and risk "low", because the policy gate had no reason to doubt it. The library already had the correct logic and the currency path bypassed it -- NumericFormatExpert parses a bare "1.200,50" as 1200.5 via _EURO_GROUPED, so adding a currency marker made the answer worse. Locale is now resolved from the value's own punctuation, most reliable evidence first: with both separators present the right-most is the decimal one (so a euro amount written the US way still reads correctly, and vice versa); a repeated separator can only be grouping; a single separator whose tail is not three digits must be decimal. Only a single separator followed by exactly three digits is genuinely ambiguous -- "1.200" is 1200 in Berlin and 1.2 in Boston -- and that is settled by the currency's documented convention rather than guessed. parse_currency_parts() exposes whether an ambiguity was resolved that way, so a caller can route the cell to review instead. Grouping is now validated too: "$1.2.3" was silently stripped to 123 and "$1,20.50" to 120.5. Both are rejected, while Indian lakh grouping ("₹1,23,456.70") stays supported. tests/test_currency_locale.py covers all of it; 6/6 European cases, both malformed-grouping cases and the end-to-end fd.clean case fail on the previous implementation, and the US/Indian cases pass on both so the fix is shown not to regress them. Default-output change: a European-format currency string in a money column now cleans to its correct magnitude. --- CHANGELOG.md | 17 ++++ src/freshdata/semantic/experts.py | 131 +++++++++++++++++++++++++++--- tests/test_currency_locale.py | 115 ++++++++++++++++++++++++++ 3 files changed, 253 insertions(+), 10 deletions(-) create mode 100644 tests/test_currency_locale.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 65491065..63153766 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,23 @@ adheres to [Semantic Versioning](https://semver.org/). ## [Unreleased] ### Fixed +- Currency parsing no longer assumes a US locale for every currency. Every + comma was deleted and the dot was taken as the decimal point regardless of + the currency present, so `"EUR 1.200,50"` read as **1.2005** — a thousand-fold + error on a monetary amount — `"€0,50"` read as **50.0**, turning fifty cents + into fifty euros, and `"€1.234.567,89"` failed to parse at all. Under + `semantic_mode="auto"` the repair was applied automatically at confidence + 0.98 and risk `low`. The locale is now resolved deterministically from the + value's own punctuation: with both separators present the right-most is the + decimal one (so a euro amount written the US way is still read correctly), + a repeated separator can only be grouping, and a single separator with a + tail that is not three digits must be decimal. Only a single separator + followed by exactly three digits is genuinely ambiguous (`"1.200"` is 1200 in + Berlin and 1.2 in Boston), and that is settled by the currency's convention + rather than guessed. Malformed grouping such as `"$1.2.3"` and `"$1,20.50"` + is now rejected instead of being coerced to `123` and `120.5`. US and Indian + lakh formats are unchanged. **Default-output change:** a European-format + currency string in a money column now cleans to its correct magnitude. - `engine="duckdb"` no longer silently changes temporal values on the fully native path (`strategy="conservative"`, `fix_dtypes=False`). A nanosecond `timedelta64[ns]` column was truncated to DuckDB's microsecond `INTERVAL` diff --git a/src/freshdata/semantic/experts.py b/src/freshdata/semantic/experts.py index f8b3aa55..c5b1fff8 100644 --- a/src/freshdata/semantic/experts.py +++ b/src/freshdata/semantic/experts.py @@ -107,8 +107,113 @@ def parse_number_words(text: str) -> int | None: ) -def parse_currency(text: str) -> float | None: - """Parse a currency-formatted string to a float, or ``None``. +#: Currencies conventionally written with a comma as the decimal separator and +#: a dot (or space) as the thousands separator. Used only to break a genuine +#: ambiguity -- a value whose own punctuation settles the question never +#: consults this table. Deliberately small: a currency absent from it and not +#: settled by structure is reported ambiguous rather than guessed. +_COMMA_DECIMAL_CURRENCIES = frozenset({"EUR", "CHF"}) + + +def _valid_grouping(part: str, sep: str) -> bool: + """Is *part* a valid thousands-grouped integer under separator *sep*? + + Grouping is only well formed when every group after the first is exactly + three digits, so ``1.234.567`` is a number and ``1.2.3`` is not. Without + this check a repeated separator would be stripped as grouping and + ``$1.2.3`` would silently read as 123. Indian lakh grouping + (``1,23,456``) is accepted as well. + """ + if sep not in part: + return True + groups = part.lstrip("+-").split(sep) + if not groups[0] or len(groups[0]) > 3: + return False + rest = groups[1:] + if not rest: + return True + if not all(g.isdigit() for g in rest): + return False + if all(len(g) == 3 for g in rest): + return True + # Indian grouping: the last group is 3 digits and the rest are 2 + # ("1,23,456.70"), which the library already accepts for INR. + return len(rest[-1]) == 3 and all(len(g) == 2 for g in rest[:-1]) + + +def _split_amount(body: str, code: str | None) -> tuple[float | None, bool]: + """Parse the numeric body of a currency string. + + Returns ``(value, ambiguous)``. ``ambiguous`` is True when the string could + be read under either convention and nothing in the input settles it, so the + caller can route the cell to a human instead of guessing. + + Resolution order, most reliable evidence first: + + 1. **Both separators present** -- the right-most is the decimal separator + and the other is grouping (``1.234,56`` and ``1,234.56`` both work). + Structure beats convention, so a euro amount written the US way is still + read correctly. + 2. **One separator, repeated** -- it must be grouping (``1.234.567``). + 3. **One separator, once, with a tail that is not 3 digits** -- it must be a + decimal separator, because grouping is always 3 digits (``12,5`` + -> 12.5, ``1.2345`` -> 1.2345). + 4. **One separator, once, with exactly 3 trailing digits** -- genuinely + ambiguous (``1.200`` is 1200 in Berlin and 1.2 in Boston). Settled by + the currency's convention when known, otherwise reported ambiguous. + """ + if not body or not any(ch.isdigit() for ch in body): + return None, False + + dots = body.count(".") + commas = body.count(",") + ambiguous = False + + if dots and commas: + decimal_sep = "." if body.rfind(".") > body.rfind(",") else "," + elif dots or commas: + sep = "." if dots else "," + occurrences = dots or commas + tail = body.rsplit(sep, 1)[1] + if occurrences > 1: + decimal_sep = "" # repeated separator can only be grouping + elif len(tail) != 3 or not tail.isdigit(): + decimal_sep = sep + elif code in _COMMA_DECIMAL_CURRENCIES: + decimal_sep = "," if sep == "," else "" + elif code is not None: + decimal_sep = "." if sep == "." else "" + else: + # No currency to appeal to and the structure does not settle it. + decimal_sep = "." if sep == "." else "" + ambiguous = True + else: + decimal_sep = "" + + group_sep = {".": ",", ",": "."}.get(decimal_sep, "." if dots else ",") + integer_part = body.split(decimal_sep, maxsplit=1)[0] if decimal_sep else body + if not _valid_grouping(integer_part, group_sep): + # e.g. "1.2.3": a repeated separator that does not form 3-digit groups + # is not a number in either convention. + return None, False + + if decimal_sep == ".": + normalized = body.replace(",", "") + elif decimal_sep == ",": + normalized = body.replace(".", "").replace(",", ".") + else: + normalized = body.replace(".", "").replace(",", "") + + if normalized in ("", "-", ".", "-."): + return None, ambiguous + try: + return float(normalized), ambiguous + except ValueError: + return None, ambiguous + + +def parse_currency_parts(text: str) -> tuple[float | None, bool]: + """``(value, ambiguous)`` for a currency string. Requires an explicit currency marker (symbol or ISO-ish code) so that a bare ``"1,200"`` is left to ordinary dtype repair, not treated as money. @@ -118,14 +223,20 @@ def parse_currency(text: str) -> float | None: codes = {t.lower() for t in re.findall(r"[A-Za-z]+", s)} has_code = bool(codes & _CURRENCY_CODES) if not (has_symbol or has_code): - return None - cleaned = re.sub(r"[A-Za-z$€£¥₹,\s]", "", s) - if cleaned in ("", "-", ".", "-."): - return None - try: - return float(cleaned) - except ValueError: - return None + return None, False + body = re.sub(r"[A-Za-z$€£¥₹\s\u00a0\u202f']", "", s) + return _split_amount(body, detect_currency(s)) + + +def parse_currency(text: str) -> float | None: + """Parse a currency-formatted string to a float, or ``None``. + + Locale is resolved deterministically from the value's own punctuation, and + only where that is genuinely ambiguous from the currency itself. See + :func:`_split_amount`. Use :func:`parse_currency_parts` when the caller + needs to know an ambiguity was resolved by convention. + """ + return parse_currency_parts(text)[0] _SYMBOL_TO_CODE = {"$": "USD", "€": "EUR", "£": "GBP", "¥": "JPY", "₹": "INR"} diff --git a/tests/test_currency_locale.py b/tests/test_currency_locale.py new file mode 100644 index 00000000..a76afa4b --- /dev/null +++ b/tests/test_currency_locale.py @@ -0,0 +1,115 @@ +"""Currency parsing must not assume a locale (FD2-002). + +Before this suite, ``parse_currency`` deleted every comma and assumed the dot +was a decimal point, whatever currency was present. ``EUR 1.200,50`` therefore +read as 1.2005 -- a thousand-fold error on a monetary amount -- and ``€0,50`` +read as 50.0, turning fifty cents into fifty euros. The repair was applied +automatically at confidence 0.98 and risk "low". + +Every case here fails on the previous implementation. +""" + +from __future__ import annotations + +import pandas as pd +import pytest + +import freshdata as fd +from freshdata.semantic.experts import parse_currency, parse_currency_parts + +# -- European formats are no longer read as US ------------------------------ + + +@pytest.mark.parametrize( + ("text", "expected"), + [ + ("EUR 1.200,50", 1200.50), + ("€1.200,50", 1200.50), + ("EUR 1.000,00", 1000.00), + ("€1.234.567,89", 1234567.89), + # The ones that were wrong in the *other* direction. + ("€0,50", 0.50), + ("EUR 12,5", 12.5), + ], +) +def test_european_amounts_parse_under_european_convention(text, expected): + assert parse_currency(text) == expected + + +@pytest.mark.parametrize( + ("text", "expected"), + [ + ("$1,200.50", 1200.50), + ("$1,234,567.89", 1234567.89), + ("₹1,200.50", 1200.50), + ("£1,200.50", 1200.50), + ("$0.50", 0.50), + ("$12.5", 12.5), + # Indian lakh grouping stays supported. + ("₹1,23,456.70", 123456.70), + ], +) +def test_us_and_indian_amounts_are_unchanged(text, expected): + assert parse_currency(text) == expected + + +def test_structure_beats_currency_convention(): + """A euro amount written the US way is still read correctly. + + The value's own punctuation is stronger evidence than the currency's + conventional format, so the convention table is consulted only when the + structure genuinely cannot settle it. + """ + assert parse_currency("€1,234.56") == 1234.56 + assert parse_currency("$1.234,56") == 1234.56 + + +@pytest.mark.parametrize("text", ["$1.2.3", "$1,20.50", "$1,2345.00", "$", "€ ,"]) +def test_malformed_grouping_is_rejected_rather_than_coerced(text): + """'1.2.3' must not be stripped to 123 by treating dots as grouping.""" + assert parse_currency(text) is None + + +@pytest.mark.parametrize("text", ["1,200", "1.200", "1.200,50", "1200"]) +def test_a_bare_number_is_still_not_currency(text): + """Unchanged contract: without a marker this is ordinary dtype repair.""" + assert parse_currency(text) is None + + +# -- ambiguity is reported, not hidden -------------------------------------- + + +def test_a_three_digit_tail_without_a_currency_is_reported_ambiguous(): + """'1.200' is 1200 in Berlin and 1.2 in Boston. + + ``parse_currency_parts`` exposes that the reading was not settled by the + input, so a caller can route the cell to a human rather than accept a + guess. (A currency marker is still required to reach this path at all.) + """ + value, ambiguous = parse_currency_parts("1.200") + assert value is None and ambiguous is False # no marker: not currency + + # With a marker the currency settles it, so it is not ambiguous. + value, ambiguous = parse_currency_parts("EUR 1.200") + assert (value, ambiguous) == (1200.0, False) + value, ambiguous = parse_currency_parts("$1,200") + assert (value, ambiguous) == (1200.0, False) + + +# -- the public API --------------------------------------------------------- + + +@pytest.mark.parametrize( + ("text", "expected"), + [("EUR 1.200,50", 1200.50), ("€0,50", 0.50), ("EUR 12,5", 12.5)], +) +def test_clean_does_not_scale_european_amounts(text, expected): + """The defect was reachable through fd.clean and applied automatically.""" + df = pd.DataFrame( + { + "k": [f"r{i}" for i in range(9)], + "amount": [10.5, 20.25, 30.0, 40.75, 50.5, 60.25, 70.0, 80.5, text], + } + ) + out = fd.clean(df, verbose=False, semantic_mode="auto") + assert out["amount"].iloc[8] == expected