diff --git a/CHANGELOG.md b/CHANGELOG.md index 6549106..6315376 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 f8b3aa5..c5b1fff 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 0000000..a76afa4 --- /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