Skip to content

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

Open
kevincostner17 wants to merge 1 commit into
mainfrom
fix/domain-validators
Open

kevincostner17 wants to merge 1 commit into
mainfrom
fix/domain-validators

Conversation

@kevincostner17

Copy link
Copy Markdown
Contributor

Summary

This PR fixes four domain-validator bugs.

  • Retail GTIN (domain='retail' turns float-loaded GTIN 4012345678901 into a different GTIN-14 #229).
    • Bug: a blank GTIN cell makes the CSV column float64, so 4012345678901 was checked as "4012345678901.0". The strip_nondigits repair then rewrote it to 40123456789010, a different GTIN-14 that still passes the mod-10 check.
    • Fix: integral float cells are now read as integer text for validation and repair. The repair skips non-integral numbers and any text with a . followed by digits, so those values are flagged but never rewritten.
  • EIDR check character (EIDR check character uses ISO 7064 Mod 37,2; real EIDR IDs fail MD-C002 #259).
    • Bug: eidr_check_char used pure ISO 7064 MOD 37-2 (with *). The EIDR ID Format spec uses the hybrid MOD 37,36 system over 0-9A-Z, so every real EIDR ID failed MD-C002.
    • Fix: the algorithm is replaced and * is no longer accepted.
    • Tests and docs: the known-answer test had been generated by the old algorithm and now uses ten published EIDR IDs. CONTRIBUTING_DOMAINS.md is corrected.
  • Finance ambiguous dates (Finance date repair guesses MM/DD for ambiguous dates that include a time #260).
    • Bug: the FIN-003 "refuse to guess DD/MM vs MM/DD" guard only matched a bare date, so 03/04/2024 09:30 bypassed it and was parsed month-first.
    • Fix: the guard now matches the numeric date prefix whatever follows it.
  • Healthcare tz handling (tz-aware vs naive datetimes crash contracts, semantic checks, domain checks and cdc_profile #233, part 2).
    • Bug: the deceased-after-birth, age-range, encounter-duration and shared ge_date checks compared offset-aware FHIR dateTime values with naive dates or mixed offsets, and raised TypeError.
    • Fix: they now parse with utc=True (naive values are read as UTC) and use pd.Timestamp.now(tz="UTC").

Behaviour changes

  • EIDR: IDs whose check character came from the old MOD 37-2 code are now flagged by MD-C002. A * check character is always rejected.
  • GTIN: float-loaded columns with integral values now validate instead of failing GS1-002. GTINs that lost a leading zero stay flagged and are not repaired.
  • Finance dates: ambiguous dates that include a time are left unresolved instead of being read month-first.
  • Date comparisons: healthcare and education checks now compare in UTC.

Tests

  • tests/domains/test_retail.py:
    • a CSV with a blank GTIN is not rewritten
    • a float GTIN column is validated as integer text
    • the repair never drops a decimal point
  • tests/domains/test_media.py:
    • ten published EIDR IDs validate
    • known-answer check characters
    • old MOD 37-2 check characters and * are rejected
  • tests/domains/test_spec_conformance.py: the published EIDR ID in row 0 is no longer flagged (MD-C002 rows are [1, 2, 3]).
  • tests/domains/test_finance.py: an ambiguous date with a time stays unresolved; an unambiguous date with a time is still repaired.
  • tests/domains/test_healthcare.py:
    • a parsed FHIR Patient with an offset deceasedDateTime
    • deceased-before-birth and implausible age with offset values
    • an Encounter with mixed offsets, compared in UTC

Verification

  • ruff check .: all checks passed
  • mypy src/freshdata: no issues found in 202 source files
  • pytest (-m "not online and not large"), py3.12 / pandas 2.3.3: 4262 passed, 6 skipped
  • pytest, py3.9 / pandas 1.5.3: 4258 passed, 10 skipped
  • Each issue's reproduction script fails on main and passes on this branch, on both lanes.

Closes #229
Closes #259
Closes #260
Refs #233 (part 2: healthcare validator)

…es, tz compares

Retail (#229): a GTIN column with a blank cell loads from CSV as float64, so
_gtin_text rendered "4012345678901.0" and GS1-002 flagged it. The
strip_nondigits repair then kept the "0" from ".0" and could write a
different, mod-10-valid GTIN-14 ("40123456789010"). Integral float cells are
now rendered as integer text for validation and repair, and the repair skips
non-integral numbers and any text with a "." followed by digits, so such
values are only flagged.

Media (#259): eidr_check_char implemented the pure ISO 7064 MOD 37-2 system,
but the EIDR ID Format spec uses the hybrid MOD 37,36 system over 0-9A-Z. All
published EIDR IDs failed MD-C002. It now uses MOD 37,36, and "*" is no longer
accepted as a check character. The known-answer test used an ID produced by
the old algorithm; it now uses ten published IDs (four registry IDs and six
examples from the EIDR ID Format document). CONTRIBUTING_DOMAINS.md is
corrected.

Finance (#260): the FIN-003 ambiguity guard was anchored at end of string, so
"03/04/2024 09:30" skipped it and was parsed month-first. The guard now matches
the numeric D/M/Y prefix with any trailing time, so such values are left
unresolved like their date-only form.

Healthcare (#233 part 2): deceased-after-birth, age-range, encounter-duration
and the shared ge_date (end >= start) check compared tz-aware FHIR dateTime
values with naive dates or mixed offsets and raised TypeError. They now parse
with utc=True (naive values read as UTC) and use pd.Timestamp.now(tz="UTC").

Closes #229
Closes #259
Closes #260
Refs #233 (part 2: healthcare validator)
@github-actions

Copy link
Copy Markdown

FreshData benchmark report — performance

  • freshdata: ?
  • python: ?
  • platform: ?
fixture n_rows n_cols p50 s p95 s peak MB repair % false-repair % preserve % trust monotonic export %

Authored-code reduction (Metric 6)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant