diff --git a/src/freshdata/fieldcheck.py b/src/freshdata/fieldcheck.py index 631395fc..e296d06b 100644 --- a/src/freshdata/fieldcheck.py +++ b/src/freshdata/fieldcheck.py @@ -71,13 +71,25 @@ class to an action. The default policy is non-destructive: nothing is deleted, _DEFAULT_NULL_MARKERS = frozenset({"", "n/a", "na", "null", "none", "nan", "-", "--"}) -_EMAIL_RE = re.compile(r"^[^@\s]+@[^@\s]+\.[A-Za-z]{2,}$") +# The TLD is ASCII letters or an internationalized TLD in punycode (xn--...). +_EMAIL_RE = re.compile(r"^[^@\s]+@[^@\s]+\.(?:[A-Za-z]{2,}|xn--[A-Za-z0-9-]+)$") _URL_RE = re.compile(r"^https?://\S+\.\S+", re.I) _TICKER_RE = re.compile(r"^[A-Z]{1,6}([.\-][A-Z0-9]{1,4})?$") -_PHONE_RE = re.compile(r"^\+?[\d\s\-().]{7,17}$") +# Phone shape: generous on formatting ("+49 (0) 30 12345678"); the real length +# check is on the digit count (_PHONE_MIN_DIGITS.._PHONE_MAX_DIGITS, per E.164). +_PHONE_RE = re.compile(r"^\+?[\d\s\-().]{7,30}$") +_PHONE_MIN_DIGITS = 7 +_PHONE_MAX_DIGITS = 15 _ID_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._\-/]*$") +def _is_phone(s: str) -> bool: + """Scalar phone check; mirrors the vectorised check in the suspect scan.""" + if not _PHONE_RE.match(s): + return False + return _PHONE_MIN_DIGITS <= sum(c.isdigit() for c in s) <= _PHONE_MAX_DIGITS + + def _safe_fullmatch(pattern: str, value: str) -> bool: """``re.fullmatch`` that never raises. @@ -126,7 +138,7 @@ def detect_value_type(value: Any) -> str: return "email" if _URL_RE.match(s): return "url" - if _PHONE_RE.match(s) and sum(c.isdigit() for c in s) >= 7: + if _is_phone(s): return "phone" return "text" @@ -514,8 +526,7 @@ def issue(classification: str, reason: str, rule: str, *, return issue("semantic_mismatch", f"{s!r} is not a valid email address", "email_format") if spec.semantic_type == "url" and not _URL_RE.match(s): return issue("semantic_mismatch", f"{s!r} is not a valid URL", "url_format") - if spec.semantic_type == "phone" and not ( - _PHONE_RE.match(s) and sum(c.isdigit() for c in s) >= 7): + if spec.semantic_type == "phone" and not _is_phone(s): return issue("semantic_mismatch", f"{s!r} is not a plausible phone number", "phone_format") if spec.max_length is not None and len(s) > spec.max_length: @@ -620,7 +631,8 @@ def _suspect_rows(series: pd.Series, spec: FieldSpec) -> pd.Index: fine &= strs.str.fullmatch(type_res[spec.semantic_type].pattern).fillna(False) elif spec.semantic_type == "phone": fine &= (strs.str.fullmatch(_PHONE_RE.pattern).fillna(False) - & (strs.str.count(r"\d") >= 7).fillna(False)) + & strs.str.count(r"\d") + .between(_PHONE_MIN_DIGITS, _PHONE_MAX_DIGITS).fillna(False)) elif spec.semantic_type in ("company_name", "entity_name", "person_name", "city", "country"): fine &= ~strs.str.fullmatch(_PURE_NUMBER_RE).fillna(False) diff --git a/src/freshdata/textclean.py b/src/freshdata/textclean.py index d0379a9a..dccd6008 100644 --- a/src/freshdata/textclean.py +++ b/src/freshdata/textclean.py @@ -231,6 +231,17 @@ def step(name: str, new: str) -> None: step(f"case_{cfg.case}", getattr(out, cfg.case)()) for name, fn in cfg.custom: step(f"custom_{name}", str(fn(out))) + if cfg.unicode_form: + # Removal steps (zero-width, control chars, punctuation, ...) can leave + # a base letter adjacent to a combining mark it could not compose with + # before, e.g. "e" + ZWSP + U+0301. Re-normalize so the output is in the + # configured form and a second pass is a no-op (#316). + form_name = f"unicode_{cfg.unicode_form.lower()}" + renormalized = unicodedata.normalize(cfg.unicode_form, out) + if renormalized != out: + out = renormalized + if form_name not in transforms: + transforms.append(form_name) if cfg.collapse_whitespace: step("collapse_whitespace", _WS_RE.sub(" ", out)) if cfg.strip: diff --git a/src/freshdata/textlint.py b/src/freshdata/textlint.py index 010e2ec3..35de0dd6 100644 --- a/src/freshdata/textlint.py +++ b/src/freshdata/textlint.py @@ -38,8 +38,13 @@ # Telltale UTF-8-decoded-as-Latin1/CP1252 sequences. _MOJIBAKE_RE = re.compile( - r"Ã[\x80-\xbf]|Â[\xa0-\xbf]|â€[\x99\x9c\x9d\x93\x94\xa6]|é|è|ü|ñ|à |’|“" + # No "Ã" + ASCII-space alternative: that matches ordinary uppercase + # Portuguese ("MANHà DE SOL"). The NBSP form ("Ã\xa0") is still covered by + # the first alternative (#317). + r"Ã[\x80-\xbf]|Â[\xa0-\xbf]|â€[\x99\x9c\x9d\x93\x94\xa6]|é|è|ü|ñ|’|“" ) +# Script-neutral letters: the ordinal indicators ª/º (as in "Nº 5", "1ª"). +_SCRIPT_NEUTRAL = frozenset({"ª", "º"}) _DATE_RE = re.compile(r"\b(\d{1,2})[/\-.](\d{1,2})[/\-.](\d{2,4})\b") _IRREGULAR_WS = {"\u00a0", "\u202f", "\u2007", "\u200b", "\u200c", "\u200d", "\ufeff"} _BIDI_MARKS = {"\u200e", "\u200f", "\u202a", "\u202b", "\u202c", "\u202d", "\u202e"} @@ -49,13 +54,18 @@ def _script_of(ch: str) -> str | None: """Coarse Unicode script family of a *letter*, or ``None`` for non-letters.""" if not ch.isalpha(): return None + # Modifier letters (Lm: the prolonged sound mark in "コーヒー", the iteration + # mark in "人々", ...) and the ordinal indicators are shared across scripts; + # treating them as their own "script" flagged ordinary text (#317). + if ch in _SCRIPT_NEUTRAL or unicodedata.category(ch) == "Lm": + return None try: name = unicodedata.name(ch) except ValueError: return None head = name.split(" ", 1)[0] # Collapse the CJK/Kana families that legitimately co-occur in Japanese. - if head in ("CJK", "HIRAGANA", "KATAKANA"): + if head in ("CJK", "HIRAGANA", "KATAKANA", "KATAKANA-HIRAGANA", "IDEOGRAPHIC"): return "CJK_JP" return head diff --git a/tests/test_text_quality_fixes.py b/tests/test_text_quality_fixes.py new file mode 100644 index 00000000..fbb34b0a --- /dev/null +++ b/tests/test_text_quality_fixes.py @@ -0,0 +1,222 @@ +"""Regression tests for text-quality fixes #316, #317 and #318.""" + +from __future__ import annotations + +import unicodedata + +import pandas as pd +import pytest + +import freshdata as fd +from freshdata.fieldcheck import FieldSpec, _is_phone, detect_value_type +from freshdata.textclean import TextCleanConfig, clean_text_value +from freshdata.textlint import _script_of + +ZWSP = "\u200b" +ACUTE = "\u0301" +JOSE_NFC = "José" + +# --------------------------------------------------------------------------- +# #316: clean_text_value output is in the configured form and idempotent +# --------------------------------------------------------------------------- + +_TRICKY = [ + "Jose\u200b\u0301", # the issue repro: ZWSP between e and acute + "e\u200c\u0301 and a\u200d\u0308", # ZWNJ / ZWJ before combining marks + "\ufeffcafe\u0301", # BOM + decomposed accent + "n\u2060\u0303", # word joiner before combining tilde + "c\x07\u0327a", # control char before combining cedilla + "a\u200e\u030a", # bidi mark before combining ring + "café\u00a0 bar", # NBSP + " Jos\u200bé \u200b", + "ᄀ\u200bᅡ", # Hangul jamo split by ZWSP + "Ａ\u200b\u0301", # fullwidth letter + "“quote” — e\u200b\u0301…", + "", + "plain ascii", +] + + +@pytest.mark.parametrize("raw", _TRICKY) +def test_clean_text_value_is_idempotent_and_nfc(raw: str) -> None: + once = clean_text_value(raw).cleaned + again = clean_text_value(once) + assert again.cleaned == once + assert again.transforms == () + assert once == unicodedata.normalize("NFC", once) + + +@pytest.mark.parametrize("form", ["NFC", "NFD", "NFKC", "NFKD"]) +@pytest.mark.parametrize("raw", _TRICKY) +def test_clean_text_value_idempotent_for_every_form(raw: str, form: str) -> None: + cfg = TextCleanConfig(unicode_form=form) + once = clean_text_value(raw, cfg).cleaned + assert clean_text_value(once, cfg).cleaned == once + assert once == unicodedata.normalize(form, once) + + +def test_issue_316_repro_composes_and_reports_form_once() -> None: + result = fd.clean_text_value("Jose" + ZWSP + ACUTE) + assert result.cleaned == JOSE_NFC + assert result.transforms == ("strip_zero_width", "unicode_nfc") + + +def test_renormalization_does_not_duplicate_transform_name() -> None: + result = clean_text_value("cafe" + ACUTE + " e" + ZWSP + ACUTE) + assert result.transforms.count("unicode_nfc") == 1 + assert result.cleaned == "café é" + + +def test_unicode_form_none_leaves_combining_sequence_alone() -> None: + cfg = TextCleanConfig(unicode_form=None) + result = clean_text_value("Jose" + ZWSP + ACUTE, cfg) + assert result.cleaned == "Jose" + ACUTE + assert result.transforms == ("strip_zero_width",) + + +# --------------------------------------------------------------------------- +# #317: lint_text_encoding false positives on ordinary text +# --------------------------------------------------------------------------- + + +def test_issue_317_repro_reports_no_issues() -> None: + df = pd.DataFrame({ + "ja": ["コーヒー"], # coffee (katakana + prolonged sound mark) + "ja2": ["人々"], # people (kanji + iteration mark) + "es": ["Nº 5"], + "pt": ["MANHà DE SOL"], + }) + assert fd.lint_text_encoding(df).issues == [] + + +@pytest.mark.parametrize("value", [ + "コーヒー", + "人々", + "スーパー", # super (katakana) + "ゝゞ", # hiragana iteration marks + "〆切", # ideographic closing mark + kanji + "Nº 5", + "1ª edição", + "2º andar", + "MANHà DE SOL", + "SÃO PAULO", + "IRMà E IRMÃO", + "LEÃO", + "Señor Muñoz", +]) +def test_ordinary_text_is_not_flagged(value: str) -> None: + rep = fd.lint_text_encoding(pd.DataFrame({"x": [value]})) + assert not [i for i in rep.issues if i.issue_type in ("mixed_script", "mojibake")] + + +@pytest.mark.parametrize("value", [ + "école", # école + "cafè", # cafè + "über", # über + "señor", # señor + "don’t", # don't + "“quoted", # "quoted + "£5", # £5 + "São Paulo", # São Paulo + "Ã\u00a0 la carte", # à la carte (NBSP form) +]) +def test_real_mojibake_is_still_detected(value: str) -> None: + rep = fd.lint_text_encoding(pd.DataFrame({"x": [value]})) + assert any(i.issue_type == "mojibake" and i.severity == "high" for i in rep.issues) + + +@pytest.mark.parametrize("value", [ + "Аlice", # Cyrillic A + "Paypal with Cyrillic а", + "Hello Ωmega", # Greek Omega +]) +def test_real_mixed_script_is_still_detected(value: str) -> None: + rep = fd.lint_text_encoding(pd.DataFrame({"x": [value]})) + assert any(i.issue_type == "mixed_script" for i in rep.issues) + + +@pytest.mark.parametrize("ch", ["ー", "々", "ª", "º", "ʰ"]) +def test_script_neutral_letters(ch: str) -> None: + assert _script_of(ch) is None + + +@pytest.mark.parametrize("ch", ["東", "は", "カ", "〆", "ヿ"]) +def test_japanese_letters_share_one_script(ch: str) -> None: + assert _script_of(ch) == "CJK_JP" + + +def test_latin_and_cyrillic_scripts_differ() -> None: + assert _script_of("a") == "LATIN" + assert _script_of("а") == "CYRILLIC" + + +# --------------------------------------------------------------------------- +# #318: international phone formats and punycode-TLD emails +# --------------------------------------------------------------------------- + + +def test_issue_318_repro_reports_no_issues() -> None: + df = pd.DataFrame({ + "p": ["+49 (0) 30 12345678", "+44 (0) 20 7946 0958", "+44 20 7946 0958"], + "e": ["user@example.xn--p1ai", "a+tag@example.com", "b@example.com"], + }) + rep = fd.validate_fields(df, {"p": "phone", "e": "email"}) + assert rep.issues == [] + + +@pytest.mark.parametrize("value", [ + "+49 (0) 30 12345678", "+44 (0) 20 7946 0958", "+1 (555) 010-9999", + "555-0100 123", "+1 415 555 0101", "+86 (0) 10 1234 5678", "+353 (0)1 234 5678", + "+1 234 567 890 12345", # 15 digits: the E.164 maximum +]) +def test_valid_phones_pass(value: str) -> None: + assert _is_phone(value) + assert detect_value_type(value) == "phone" + rep = fd.validate_fields(pd.DataFrame({"p": [value]}), + {"p": FieldSpec(semantic_type="phone")}) + assert rep.issues == [] + + +@pytest.mark.parametrize("value", [ + "12345", # too few digits + "123-45", + "+1234567890123456", # 16 digits: over the E.164 maximum + "+1 (555) 010-9999 ext 4", # letters + "(((((((((((())))))", # no digits + "+" + "1 " * 16, # 16 digits spread out + "(" * 31, # too long overall +]) +def test_invalid_phones_rejected(value: str) -> None: + assert not _is_phone(value) + rep = fd.validate_fields(pd.DataFrame({"p": [value]}), + {"p": FieldSpec(semantic_type="phone")}) + assert [i.rule for i in rep.issues] == ["phone_format"] + + +def test_phone_checks_agree_across_a_mixed_column() -> None: + values = ["+49 (0) 30 12345678", "12345", "+44 20 7946 0958", "+1234567890123456", + "+33 (0)1 23 45 67 89", "hello world"] + rep = fd.validate_fields(pd.DataFrame({"p": values}), + {"p": FieldSpec(semantic_type="phone")}) + assert sorted(i.row for i in rep.issues) == [1, 3, 5] + + +@pytest.mark.parametrize("value", [ + "user@example.xn--p1ai", "user@xn--80ak6aa92e.xn--p1ai", "a@b.co", + "first.last+tag@sub.example.org", "x@example.xn--fiqs8s", +]) +def test_valid_emails_pass(value: str) -> None: + assert detect_value_type(value) == "email" + rep = fd.validate_fields(pd.DataFrame({"e": [value]}), + {"e": FieldSpec(semantic_type="email")}) + assert rep.issues == [] + + +@pytest.mark.parametrize("value", [ + "a@b.c", "a@b.xn--", "a@b.com1", "x@y", "no-at-sign.com", "a b@c.com", + "a@b.xn--p1ai!", +]) +def test_invalid_emails_rejected(value: str) -> None: + rep = fd.validate_fields(pd.DataFrame({"e": [value]}), + {"e": FieldSpec(semantic_type="email")}) + assert [i.rule for i in rep.issues] == ["email_format"]