fix(domains): GTIN float rewrite, EIDR check character, ambiguous dates with time, healthcare tz

CONTRIBUTING_DOMAINS.md

@@ -172,6 +172,6 @@ key so the repair stays config-driven.
 ### Tested pure check-digit functions
 
 Identifier check digits are pure, separately unit-tested functions: see
-`eidr_check_char` / `is_valid_eidr` (ISO 7064 Mod 37,2) and `is_valid_icpn`
+`eidr_check_char` / `is_valid_eidr` (ISO 7064 hybrid MOD 37,36) and `is_valid_icpn`
 (GS1 mod-10 for UPC/EAN) in `media/validator.py`, anchored by a published known-answer
 plus round-trip and tamper tests.

src/freshdata/domains/_common.py

@@ -293,8 +293,11 @@ def check_at_least_one(df: pd.DataFrame, mapping: ColumnMapping, rule: Rule) ->
 
 def check_ge_date(df: pd.DataFrame, mapping: ColumnMapping, rule: Rule) -> list[Any]:
     """Flag rows where ``fields[0]`` is earlier than ``fields[1]`` (``fields[0] >= fields[1]``)."""
-    later = to_datetime_safe(df[mapping.actual(rule.fields[0])])
-    earlier = to_datetime_safe(df[mapping.actual(rule.fields[1])])
+    # utc=True: one side may carry a UTC offset (e.g. a FHIR dateTime) while the other
+    # is naive or uses a different offset; comparing in UTC (naive read as UTC) never
+    # raises and does not change the ordering of two naive values.
+    later = to_datetime_safe(df[mapping.actual(rule.fields[0])], utc=True)
+    earlier = to_datetime_safe(df[mapping.actual(rule.fields[1])], utc=True)
     both = later.notna() & earlier.notna()
     return df.index[both & (later < earlier)].tolist()
 

src/freshdata/domains/finance/validator.py

@@ -27,8 +27,10 @@
 _PACK_DIR = Path(__file__).resolve().parent
 _ISO_DATE_RE = re.compile(r"\d{4}-\d{2}-\d{2}")
 # Numeric date with both leading components <= 12 is genuinely ambiguous
-# (could be DD/MM or MM/DD), so we refuse to coerce it.
-_AMBIGUOUS_DATE_RE = re.compile(r"^\s*(\d{1,2})[/.-](\d{1,2})[/.-](\d{2,4})\s*$")
+# (could be DD/MM or MM/DD), so we refuse to coerce it. Anything after the year
+# (a time such as "03/04/2024 09:30", a "T" separator, a comma) keeps the date part
+# just as ambiguous, so only the numeric date prefix is matched.
+_AMBIGUOUS_DATE_RE = re.compile(r"^\s*(\d{1,2})[/.-](\d{1,2})[/.-](\d{2,4})(?!\d)")
 
 
 @lru_cache(maxsize=1)

src/freshdata/domains/healthcare/validator.py

@@ -406,8 +406,11 @@ def _check_gender(self, df: pd.DataFrame, mapping: ColumnMapping, rule: Rule) ->
     def _check_deceased_after_birth(
         self, df: pd.DataFrame, mapping: ColumnMapping, rule: Rule
     ) -> list[Any]:
-        deceased_date = to_datetime_safe(df[mapping.actual("deceased_date")])
-        birth_date = to_datetime_safe(df[mapping.actual("birth_date")])
+        # utc=True: FHIR dateTime values may carry a UTC offset while birthDate is a
+        # plain date; normalising both to UTC (naive values read as UTC) keeps the
+        # comparison from raising on tz-aware vs naive or mixed offsets.
+        deceased_date = to_datetime_safe(df[mapping.actual("deceased_date")], utc=True)
+        birth_date = to_datetime_safe(df[mapping.actual("birth_date")], utc=True)
         deceased_col = mapping.actual("deceased")
         truthy = (
             self._truthy(df[deceased_col]) if deceased_col is not None
@@ -419,8 +422,8 @@ def _check_deceased_after_birth(
     def _check_age_range(
         self, df: pd.DataFrame, mapping: ColumnMapping, rule: Rule
     ) -> list[Any]:
-        birth_date = to_datetime_safe(df[mapping.actual("birth_date")])
-        age_years = (pd.Timestamp.now() - birth_date).dt.days / _DAYS_PER_YEAR
+        birth_date = to_datetime_safe(df[mapping.actual("birth_date")], utc=True)
+        age_years = (pd.Timestamp.now(tz="UTC") - birth_date).dt.days / _DAYS_PER_YEAR
         bad = birth_date.notna() & ((age_years < 0) | (age_years > _MAX_AGE_YEARS))
         return df.index[bad].tolist()
 
@@ -474,8 +477,8 @@ def _check_not_future_finished(
     def _check_duration(
         self, df: pd.DataFrame, mapping: ColumnMapping, rule: Rule
     ) -> list[Any]:
-        start = to_datetime_safe(df[mapping.actual("period_start")])
-        end = to_datetime_safe(df[mapping.actual("period_end")])
+        start = to_datetime_safe(df[mapping.actual("period_start")], utc=True)
+        end = to_datetime_safe(df[mapping.actual("period_end")], utc=True)
         finished = self._is_finished(df, mapping)
         both = start.notna() & end.notna() & finished
         return df.index[both & ((end - start).dt.days >= _MAX_ENCOUNTER_DAYS)].tolist()

src/freshdata/domains/media/validator.py

@@ -5,7 +5,7 @@
 When ``media_type`` is omitted it is auto-detected from the column signature; an
 indeterminate signature raises :class:`AmbiguousMediaTypeError`.
 
-The EIDR DOI check character (ISO 7064 Mod 37,2) and the ICPN (UPC/EAN) GS1 mod-10 check
+The EIDR DOI check character (ISO 7064 hybrid MOD 37,36) and the ICPN (UPC/EAN) GS1 mod-10 check
 digit are implemented here as pure, unit-tested functions.
 """
 
@@ -47,9 +47,13 @@ def _sort_key(value: Any) -> tuple[int, Any]:
         return (0, value)
     return (1, str(value))
 _EIDR_RE = re.compile(
-    r"10\.5240/([0-9A-Z]{4})-([0-9A-Z]{4})-([0-9A-Z]{4})-([0-9A-Z]{4})-([0-9A-Z]{4})-([0-9A-Z*])"
+    r"10\.5240/([0-9A-Z]{4})-([0-9A-Z]{4})-([0-9A-Z]{4})-([0-9A-Z]{4})-([0-9A-Z]{4})-([0-9A-Z])"
 )
-_ISO7064_MOD = 37
+# ISO/IEC 7064 hybrid system MOD 37,36: modulus M = 36 over the alphabet 0-9A-Z,
+# with M + 1 = 37 as the second modulus. The EIDR ID Format specification names
+# "ISO 7064 Mod 37,36"; the pure MOD 37-2 system (with the supplementary ``*``) is
+# a different algorithm and rejects real EIDR IDs.
+_ISO7064_M = 36
 
 
 # -- pure check-digit functions (unit-tested in tests/domains/test_media.py) --
@@ -60,27 +64,24 @@ def _char_value(char: str) -> int:
 
 
 def _value_char(value: int) -> str:
-    """Inverse of :func:`_char_value`; 36 maps to the ISO 7064 supplementary ``*``."""
-    if value < 10:
-        return chr(48 + value)
-    if value < 36:
-        return chr(55 + value)
-    return "*"
+    """Inverse of :func:`_char_value` for values 0-35."""
+    return chr(48 + value) if value < 10 else chr(55 + value)
 
 
 def eidr_check_char(payload: str) -> str:
-    """Return the EIDR check character for *payload* via ISO 7064 Mod 37,2.
+    """Return the EIDR check character for *payload* via ISO 7064 hybrid MOD 37,36.
 
     *payload* is the 20-character DOI suffix (hyphens removed, check char excluded).
     """
-    remainder = 0
+    product = _ISO7064_M
     for char in payload:
-        remainder = (remainder + _char_value(char)) * 2 % _ISO7064_MOD
-    return _value_char((_ISO7064_MOD + 1 - remainder) % _ISO7064_MOD)
+        total = (product + _char_value(char)) % _ISO7064_M or _ISO7064_M
+        product = total * 2 % (_ISO7064_M + 1)
+    return _value_char((_ISO7064_M + 1 - product) % _ISO7064_M)
 
 
 def is_valid_eidr(value: Any) -> bool:
-    """True if *value* is a well-formed EIDR DOI with a valid Mod 37,2 check character."""
+    """True if *value* is a well-formed EIDR DOI with a valid MOD 37,36 check character."""
     if not isinstance(value, str):
         return False
     match = _EIDR_RE.fullmatch(value.strip())

src/freshdata/domains/retail/validator.py

@@ -10,11 +10,13 @@
 from __future__ import annotations
 
 import json
+import math
 import re
 from functools import lru_cache
 from pathlib import Path
 from typing import Any
 
+import numpy as np
 import pandas as pd
 
 from ..base import ColumnMapping, ConfigDrivenValidator, Rule, RuleResult
@@ -23,6 +25,7 @@
 _BUNDLED_DIR = _PACK_DIR.parent / "bundled"
 _GTIN_LENGTHS = (8, 12, 13, 14)
 _NONDIGIT = re.compile(r"\D")
+_DECIMAL_POINT = re.compile(r"\.\d")
 
 
 @lru_cache(maxsize=1)
@@ -56,6 +59,22 @@ def _gtin_well_formed(text: str) -> bool:
     return text.isdigit() and len(text) in _GTIN_LENGTHS
 
 
+def _integral_float_text(value: Any) -> Any:
+    """Render an integral float cell as integer text (``4012345678901.0`` -> ``"4012345678901"``).
+
+    A GTIN column with a blank cell loads from CSV as float64; its ``str()`` form
+    carries a ``.0`` suffix whose ``0`` would otherwise be read as an extra digit.
+    Every other value is returned unchanged.
+    """
+    if (
+        isinstance(value, (float, np.floating))
+        and math.isfinite(value)
+        and float(value).is_integer()
+    ):
+        return str(int(value))
+    return value
+
+
 class RetailValidator(ConfigDrivenValidator):
     """Validator for GS1-aligned product catalog frames."""
 
@@ -109,7 +128,7 @@ def reference_sources(self) -> list[dict[str, Any]]:
 
     def _gtin_text(self, df: pd.DataFrame, mapping: ColumnMapping) -> tuple[pd.Series, pd.Series]:
         series = df[mapping.actual("gtin")]
-        return series, series.astype("string").str.strip()
+        return series, series.map(_integral_float_text).astype("string").str.strip()
 
     def _check_gtin_length(
         self, df: pd.DataFrame, mapping: ColumnMapping, rule: Rule
@@ -172,7 +191,12 @@ def _repair_strip_nondigits(
             value = df.at[row, col]
             if pd.isna(value):
                 continue
-            digits = _NONDIGIT.sub("", str(value))
+            text = _integral_float_text(value)
+            if not isinstance(text, str):
+                continue  # a non-integral number is not a GTIN with separators
+            if _DECIMAL_POINT.search(text):
+                continue  # "4012345678901.0": dropping the "." would invent a digit
+            digits = _NONDIGIT.sub("", text)
             if _gtin_well_formed(digits) and _mod10_valid(digits):
                 fixes[row] = digits
         return fixes

tests/domains/test_finance.py

@@ -209,3 +209,24 @@ def test_balance_check_survives_all_missing_transaction_ids(good_finance):
     _, rep2 = fd.clean(df2, domain="finance", return_report=True, verbose=False)
     fin006 = next(f for f in rep2.domain_findings if f["rule_id"] == "FIN-006")
     assert fin006["n_violations"] == 3
+
+
+@pytest.mark.parametrize("value", ["03/04/2024 09:30", "03-04-2024T09:30:00"])
+def test_ambiguous_date_with_time_not_silently_coerced(good_finance, value):
+    # Regression (#260): a trailing time used to bypass the DD/MM vs MM/DD guard,
+    # so "03/04/2024 09:30" was silently read month-first as 2024-03-04.
+    df = good_finance.copy()
+    df.loc[0, "date"] = value
+    out, rep = fd.clean(df, domain="finance", return_report=True, verbose=False)
+    assert out.loc[0, "date"] == value
+    assert any(r["rule_id"] == "FIN-003" and r["status"] == "unresolvable"
+               for r in rep.domain_repairs)
+
+
+def test_unambiguous_date_with_time_still_coerced(good_finance):
+    df = good_finance.copy()
+    df.loc[0, "date"] = "01/15/2024 09:30"   # 15 can't be a month
+    out, rep = fd.clean(df, domain="finance", return_report=True, verbose=False)
+    assert out.loc[0, "date"] == "2024-01-15"
+    assert any(r["rule_id"] == "FIN-003" and r["status"] == "applied"
+               and r["to"] == "2024-01-15" for r in rep.domain_repairs)

tests/domains/test_healthcare.py

@@ -351,3 +351,43 @@ def test_unknown_domain_lists_available(good_patient):
 
 def test_standalone_import():
     assert HealthcareValidator(fhir_resource="Patient").domain_name == "healthcare"
+
+
+# -- timezone handling (#233 part 2) -----------------------------------------
+
+def test_parsed_fhir_patient_with_offset_deceased_datetime_validates():
+    # FHIR dateTime keeps its UTC offset while birthDate is a plain date; comparing
+    # the two used to raise "Invalid comparison between ... and DatetimeArray".
+    resource = {"resourceType": "Patient", "id": "1", "birthDate": "1970-01-01",
+                "gender": "male", "deceasedDateTime": "2015-02-14T13:42:00+10:00"}
+    frame = fd.parse_domain(resource, format="fhir").frames["patient"]
+    _, rep = fd.clean(frame, domain="healthcare", return_report=True, verbose=False)
+    assert not _violated(rep, "HC-P006")
+
+
+def test_deceased_before_birth_detected_with_offset_datetime(good_patient):
+    df = good_patient.copy()
+    df["deceased_date"] = [None, None, "1950-01-01T10:00:00+10:00"]   # before 1955 birth
+    _, rep = fd.clean(df, domain="healthcare", fhir_resource="Patient",
+                      return_report=True, verbose=False)
+    assert _violated(rep, "HC-P006")
+
+
+def test_implausible_age_with_offset_birth_datetime(good_patient):
+    df = good_patient.copy()
+    df.loc[0, "birth_date"] = "1820-01-01T00:00:00+05:00"   # aware among naive dates
+    _, rep = fd.clean(df, domain="healthcare", fhir_resource="Patient",
+                      return_report=True, verbose=False)
+    assert _violated(rep, "HC-P009")
+
+
+def test_encounter_mixed_offsets_compare_in_utc(good_encounter):
+    df = good_encounter.copy()
+    # E1: 08:00Z -> 07:30Z ends before it starts in UTC (wall-clock would look fine).
+    # E2: aware start with a naive end more than 365 days later.
+    df["period_start"] = ["2024-03-10T03:00:00-05:00", "2023-01-01T00:00:00+10:00"]
+    df["period_end"] = ["2024-03-10T03:30:00-04:00", "2024-06-01"]
+    _, rep = fd.clean(df, domain="healthcare", fhir_resource="Encounter",
+                      return_report=True, verbose=False)
+    assert _violated(rep, "HC-E005")
+    assert _violated(rep, "HC-E008")

tests/domains/test_media.py

@@ -69,10 +69,48 @@ def good_release() -> pd.DataFrame:
 
 # -- pure check-digit functions -------------------------------------------
 
+# Published EIDR IDs: the first four are real registry IDs, the rest are the examples
+# printed in the EIDR ID Format document. Their check characters use ISO 7064 hybrid
+# MOD 37,36, the system the spec names ("C is the ISO 7064 Mod 37,36 check character").
+PUBLISHED_EIDRS = [
+    "10.5240/7791-8534-2C23-9030-8610-5",
+    "10.5240/B752-5B47-DBBE-E5D4-5A3F-N",
+    "10.5240/0EF3-54F9-2642-0B49-6829-R",
+    "10.5240/1489-49A2-3956-4B2D-FE16-5",
+    "10.5240/F85A-E100-B068-5B8F-B1C8-T",
+    "10.5240/3466-F12C-391A-D60B-206B-Y",
+    "10.5240/CA51-02D0-3269-23C9-DB5A-E",
+    "10.5240/7481-838B-59CA-63D0-B9A8-E",
+    "10.5240/CE43-9B6A-2C41-35C3-42CA-V",
+    "10.5240/823E-5DE9-0816-7BB5-A37F-X",
+]
+
+
+@pytest.mark.parametrize("eidr", PUBLISHED_EIDRS)
+def test_eidr_published_ids_are_valid(eidr):
+    assert is_valid_eidr(eidr)
+    payload = eidr[len("10.5240/"):].replace("-", "")[:-1]
+    assert eidr_check_char(payload) == eidr[-1]
+
+
 def test_eidr_check_char_matches_published_example():
-    # EIDR's canonical published identifier and its check character '7'.
-    assert is_valid_eidr("10.5240/7791-8534-2C23-9030-8004-7")
-    assert eidr_check_char("779185342C2390308004") == "7"
+    # EIDR's canonical published identifier and its check character '5'.
+    assert eidr_check_char("779185342C2390308610") == "5"
+    assert eidr_check_char("F85AE100B0685B8FB1C8") == "T"
+
+
+def test_eidr_old_mod_37_2_check_chars_are_rejected():
+    # Regression (#259): the pure MOD 37-2 system gave '7' here and could emit '*';
+    # neither is a valid EIDR check character.
+    assert not is_valid_eidr("10.5240/7791-8534-2C23-9030-8004-7")
+    assert eidr_check_char("779185342C2390308004") == "C"
+    assert not is_valid_eidr("10.5240/7791-8534-2C23-9030-8610-*")
+
+
+def test_eidr_check_char_is_alphanumeric():
+    for payload in ("00000000000000000000", "ZZZZZZZZZZZZZZZZZZZZ", "0123456789ABCDEFGHIJ"):
+        char = eidr_check_char(payload)
+        assert len(char) == 1 and char.isalnum() and char == char.upper()
 
 
 def test_eidr_roundtrip_and_tamper():

tests/domains/test_retail.py

@@ -2,6 +2,8 @@
 
 from __future__ import annotations
 
+import io
+
 import pandas as pd
 import pytest
 
@@ -139,3 +141,43 @@ def test_unknown_domain_lists_available(good_retail):
 
 def test_standalone_import():
     assert RetailValidator().domain_name == "retail"   # importable on its own (top of file)
+
+
+# -- float-loaded GTINs (#229) ---------------------------------------------
+
+def _mod10_ok(code: str) -> bool:
+    return _check_digit(code[:-1]) == code[-1]
+
+
+def test_csv_gtin_with_blank_cell_is_not_rewritten():
+    # A blank GTIN cell makes the CSV column float64 ("4012345678901.0"); the ".0"
+    # used to be stripped to "40123456789010", a different but mod-10-valid GTIN-14.
+    df = pd.read_csv(io.StringIO("gtin,product_description\n4012345678901,Widget\n,Missing\n"))
+    assert df["gtin"].dtype == "float64"
+    assert _mod10_ok("40123456789010")   # why the old rewrite looked legitimate
+    out, rep = fd.clean(df, domain="retail", return_report=True, verbose=False)
+    assert out["gtin"].iloc[0] == 4012345678901
+    assert not _violated(rep, "GS1-002")
+    assert not _violated(rep, "GS1-003")
+    assert not [r for r in rep.domain_repairs
+                if r["rule_id"] == "GS1-002" and r["status"] == "applied"]
+
+
+def test_float_gtin_column_is_validated_as_integer_text():
+    df = pd.DataFrame({"gtin": [4012345678901.0, float("nan"), 4012345678902.0]})
+    report = RetailValidator().validate(df)
+    by_id = {r.rule_id: r for r in report.results}
+    assert not by_id["GS1-002"].violated            # 13 digits once ".0" is dropped
+    assert by_id["GS1-003"].violation_rows == [2]   # wrong check digit is still caught
+
+
+@pytest.mark.parametrize("value", ["4012345678901.0", 4012345678901.5])
+def test_strip_nondigits_never_drops_a_decimal_point(good_retail, value):
+    df = good_retail.copy()
+    df["gtin"] = df["gtin"].astype(object)
+    df.loc[0, "gtin"] = value
+    validator = RetailValidator()
+    report = validator.validate(df)
+    assert any(r.rule_id == "GS1-002" and r.violated for r in report.results)
+    out, _ = validator.repair(df, report)
+    assert out.loc[0, "gtin"] == value   # flagged, never rewritten

tests/domains/test_spec_conformance.py

@@ -305,6 +305,10 @@ def test_media_eidr_content_conformance():
                       return_report=True, verbose=False)
     assert _violated(rep, "MD-C001")            # null eidr_id
     assert _violated(rep, "MD-C002")            # invalid EIDR DOI format
+    c002 = next(f for f in rep.domain_findings if f["rule_id"] == "MD-C002")
+    # Row 0 is a published EIDR ID (valid ISO 7064 MOD 37,36 check char); rows 1-3
+    # are malformed, carry the wrong DOI prefix, or have a bad check character.
+    assert c002["violation_rows"] == [1, 2, 3]
     assert _violated(rep, "MD-C005")            # invalid country ZZ
     assert _violated(rep, "MD-C006")            # invalid language 'xx'