From 6d5f963a21628c3a91355f3a10b5e2b8cba0771d Mon Sep 17 00:00:00 2001 From: Ethan Ligon Date: Tue, 21 Jul 2026 13:13:16 -0700 Subject: [PATCH 1/3] fix(Malawi): 2004-05's cluster key was an intra-TA sequence number (GH #323, config-only) Malawi's `cluster_features` is declared (t, v) but every wave extracts it from the household cover page, so it is projected from household grain onto the cluster with groupby().first() -- one arbitrary household's answer, served as the cluster's. Measured cold, the five waves turn out to have two different diseases, and only one of them is a broken key. 2004-05 (IHS2) -- BROKEN KEY. `ea` is the EA's sequence number WITHIN its Traditional Authority: 110 distinct values for 564 enumeration areas, so ~5 unrelated EAs merged into one `v`. `psu`, already a column in the same file, is the fully qualified 8-digit code (region+district+TA+EA): 564 values, exactly 20 households each, zero contested cells. Contested cells 164 -> 0; clusters 110 -> 564; districts in cluster_features 20 -> 26; and the settlement strata go from {Rural} -- every one of the 110 mega-clusters came out rural -- back to {Rural, Urban}. Declared identically in `sample` and `cluster_features`, as _join_v_from_sample requires. 2013-14 / 2019-20 -- NOT a broken key. `ea_id` is the IHS3 baseline EA and the IHPS TRACKS movers. The GPS settles it: 188 of 204 EAs in 2013-14 have households at different coordinates, but restrict to households that did not move and all 204 collapse to exactly one coordinate. Every kilometre of within-EA spread (median 145 km, max 775 km) is a tracked mover. Re-keying on current district would give 626 groups of median ONE household and sever the panel from its baseline, so these residuals are reported, not forced to zero. Also fixed, en route: 2016-17 had GPS for 0 of its 880 clusters because df_geo declared the raw case_id while df_main runs it through `cs_i` -- the merge key never matched and the outer join produced 12,447 orphan rows. The files are 1:1 on case_id, so mirroring the transform makes the join exact: 779 of 880 clusters now carry coordinates. And 2016-17's one Region "conflict" was a spelling (Southern vs South for an EA sampled in both halves); the existing `region` table is now applied at extraction. Site 4 is NOT touched: 2010-11 and 2019-20 merge a household-grain geo file on the cluster key, 183,812 and 171,230 phantom rows. The one-line config fix was implemented and measured -- contested cells 0 and 250, identical either way, confirming that a cartesian cannot change a contested-cell count -- then reverted so PR #627's cardinality guard keeps these cells as evidence. Documented in place instead. No file under lsms_library/*.py. No `aggregation:` key, no declared reducer, no value blanked to . Refs #323. Co-Authored-By: Claude Opus 4.8 (1M context) --- .coder/ledger/323-malawi-key.md | 236 ++++++++++++++ .../countries/Malawi/2004-05/_/data_info.yml | 28 +- .../countries/Malawi/2010-11/_/data_info.yml | 17 + .../countries/Malawi/2016-17/_/data_info.yml | 27 +- .../countries/Malawi/2019-20/_/data_info.yml | 18 ++ lsms_library/countries/Malawi/_/CONTENTS.org | 132 +++++++- tests/test_gh323_malawi_cluster_key.py | 299 ++++++++++++++++++ 7 files changed, 752 insertions(+), 5 deletions(-) create mode 100644 .coder/ledger/323-malawi-key.md create mode 100644 tests/test_gh323_malawi_cluster_key.py diff --git a/.coder/ledger/323-malawi-key.md b/.coder/ledger/323-malawi-key.md new file mode 100644 index 000000000..063893dd2 --- /dev/null +++ b/.coder/ledger/323-malawi-key.md @@ -0,0 +1,236 @@ +# Prior-Art Ledger — GH #323 (Malawi, Site 2): the cluster key + +**Search tier used:** ripgrep + git, plus PR #634 / `.coder/ledger/323-uganda.md` +(the template for this task), the consolidation note +`slurm_logs/DESIGN_grain_collapse_sites_2026-07-13.org` (branch +`origin/docs/323-grain-collapse-sites`, Site 2 + D1/D2/D3), and PR #627's Site-4 +cartesian census. Every number below is a **cold** build against an *isolated* +`LSMS_DATA_DIR` (fresh tree, `dvc-cache` symlinked to the shared L1) with +`LSMS_COUNTRIES_ROOT` pointed at this worktree and asserted before any run. +`LSMS_NO_CACHE=1` alone was **not** trusted — it is soft for script-path L2-wave +parquets, and the collapse is baked into the cache it poisoned. + +## §1 Task, restated + +Malawi's `cluster_features` is declared `(t, v)` in `_/data_scheme.yml` but every +one of the five waves extracts it from the household **cover page**, so the frame +arrives at household grain and is projected onto the cluster by +`Wave.cluster_features` → `country._collapse_to_cluster_grain` → `.first()`. Where +the households of one `v` disagree about the cluster's own attributes, one +arbitrary household's answer is served as the cluster's — and `.first()` skips NA +per column, so the row it returns can be a **composite existing in no source +record**. The task: find, empirically, what actually identifies a cluster in each +wave and fix the identifier. Config only (`countries/Malawi/**`); no +`lsms_library/*.py`, no `aggregation:` keys, no declared reducer, no blanking to +``. + +## §2 Existing machinery (this task's area) + +| symbol | path | what it does | tested? | reuse / extend / new | +|--------|------|--------------|---------|----------------------| +| `_collapse_to_cluster_grain` | `country.py:4490` | Site 2's audit-then-`.first()`; emits `GrainCollapseWarning` | yes | **do not touch** (core, PR #617) — monkeypatched *read-only* to capture the pre-collapse frame | +| `_join_v_from_sample` | `country.py` | joins `cluster_features` onto household tables through `sample.v` | yes | the constraint: forces `sample.v` ≡ `cluster_features.v` | +| `_enforce_canonical_spellings` | `country.py` (`_finalize_result`) | `rural`/`RURAL` → `Rural` at API time | yes | **reuse** — the reason no `Rural` mapping is needed in YAML | +| auto-discovery categorical mapping | `country.py` (`_apply_categorical_mappings`) | column name ↔ `categorical_mapping.org` table name, case-insensitive | yes | **reuse**, and *also* apply the `region` table at extraction (§6.3) | +| `mappings: [table, from, to]` in `myvars` | `country.py:760` `map_formatting_function` | pulls a `categorical_mapping.org` table into a dict | yes (used by Malawi `strata` in all 5 waves) | **reuse** for 2016-17 `Region` | +| `psu` column | `Malawi/2004-05/Data/sec_a.dta` | the fully qualified 8-digit EA code, already in the file | — | **reuse** — no derivation, no composite, no new primitive | +| `cs_i` | `Malawi/2016-17/_/mapping.py`, `2019-20/_/mapping.py` | `'cs-17-' + format_id(...)`, keeps CS ids apart from panel `y{3,4}_hhid` | — | **reuse** — `df_geo` must build `i` the same way `df_main` does | +| `Rural: 0/1` inline `mapping:` blocks | all 5 waves | **already removed on `development`** by GH #602 | yes | nothing to do — do **not** re-add | +| `aggregation:` block | `Malawi/_/data_scheme.yml` (`interview_date`) | dead config; nothing reads it | n/a | **forbidden** to add more (D1); the existing one is out of scope | + +## §3 Definitions & conventions in force + +- `sample()` is the single source of truth for a household's cluster; `v` is + joined from it at API time — `CLAUDE.md` "`sample()` and Cluster Identity". + ⇒ **`sample.v` and `cluster_features.v` must be the same key.** +- `cluster_features` owns `v`; index `(t, v)` — `Malawi/_/data_scheme.yml`. +- Canonical `Rural` is `str` with spellings `Rural: [rural, RURAL, …]` — + `lsms_library/data_info.yml`. **Not** a 0/1 indicator. +- "NO AGGREGATION IN CORE" — `SkunkWorks/grain_aggregation_policy.org`, D1 of the + consolidation note. +- D2: NaN-key rows are deleted and reported, never retained. (Malawi has none: + every wave's cover page populates its cluster column for every household.) +- D3: Sites 2 and 4 get their own PRs. Site 4 is PR #627 — see §6.5. + +## §4 Invariants & assumptions + +- **The Malawi EA code is structured**: `region(1) + district(2) + TA(2) + EA(3)`, + 8 digits. Verified by decoding: the leading digit reproduces the reported + region for 11,280/11,280 households in 2004-05. +- **Frame vintage is not shared across the IHS2/IHS3 boundary.** 2004-05's 564 + EA codes overlap 2010-11's 768 in only **3** codes (1998 vs 2008 census frame). + The *format* is common to all five waves; the *sets* are not. So a cross-wave + join on `v` is meaningful from 2010-11 on and not before — and the 2004-05 fix + does not, and cannot, create a spurious panel link. +- **The IHPS waves are TRACKED panels.** `ea_id` in 2013-14 / the panel halves of + 2016-17 and 2019-20 is the **IHS3 baseline EA**, and the panel EA sets nest + cleanly: 2019-20-PN = 2016-17-PN = 102 ⊂ 2013-14 = 204 ⊂ 2010-11 = 768. So a + cluster's households genuinely live in different districts. Dispersal, not + collision — see §6.2. +- **The two halves of 2016-17 / 2019-20 do not collide.** `ea_id` is one national + keyspace; the single code shared between the 2016-17 CS and panel halves + (`30305580`) is the *same real EA* in the *same district*, sampled twice. +- `format_id` is auto-applied to `idxvars`, not `myvars` — `CLAUDE.md`. `psu` is + already an 8-character string in the source, so `sample.myvars.v: psu` needs no + formatter. +- **A cartesian merge cannot change a contested-cell count.** `nunique(dropna=True)` + per (cluster, column) is invariant to row repetition. Assumed by PR #627's + owner; **measured** here rather than assumed — §7. + +## §5 Reuse decision + +| quantity | decision | reason | +|----------|----------|--------| +| 2004-05 cluster key | **reuse the source's own `psu`** | Not a composite, not derived: the fully qualified code is already a column. 564 values, exactly 20 HH each, 0 contested cells. A `(dist, ta, ea)` composite would produce the identical partition (verified: 564 groups) with three columns instead of one, and would *not* be on the later waves' 8-digit keyspace. | +| 2013-14 / 2019-20 panel keys | **no change** | The contest is dispersal. Uganda §6.5 set the precedent (`comm` not re-keyed): a structured code with zero collisions in its frame year, whose multi-district groups show a dispersal signature, must not be split by current district. Splitting 2013-14 on `(district, ea_id)` yields 626 groups of **median 1 household** — 422 singleton pseudo-clusters — and would desynchronise `v` from the IHS3 baseline. | +| GPS reducer | **none** — no averaging, no median | Same argument that retired core's `.mean()` on 2026-07-13, and here it is decisive: the 2013-14 within-EA spread is *entirely* movers (§6.2), so a centroid would place a cluster in the middle of nowhere between an EA and the towns its emigrants moved to. | +| categorical reducer | **none declared** | D1. The key fix removes the conflicts that were fixable; the rest are reported. | +| `Rural` 0/1 mapping | **nothing** — already removed by GH #602 on `development` | A second implementation would fork the vocabulary. Canonical `spellings` already normalise the three cases. | +| 2016-17 `Region` | **extend**: apply the existing `region` table at extraction | The auto-discovery mapping already collapses `South`/`Southern` at API time, so this is output-neutral; it just stops the *collapse* from reading a spelling difference as a disagreement about the cluster. | +| 2016-17 `df_geo` merge key | **fix**: build `i` with `cs_i`, exactly as `df_main` does | Not a new mechanism — the transform already exists and `df_main` already uses it. The block simply failed to mirror it. | +| 2010-11 / 2019-20 cartesian `df_geo` merges | **leave, and document in place** | Site 4, PR #627 owns it (D3). The one-line config fix was implemented, measured, and then **reverted** so #627's cardinality guard keeps these two cells as evidence. §6.5. | + +## §6 Decisions, with the evidence + +1. **2004-05's `ea` is not a cluster id — it is an intra-TA sequence number.** + 110 distinct values for 564 EAs. Measured at source, contested cells by key: + + | key | groups | region | district | reside | + |-----|--------|--------|----------|--------| + | `ea` | 110 | 66 | 79 | 19 | + | `(dist, ea)` | 447 | 0 | 0 | 8 | + | `(dist, ta, ea)` | 564 | 0 | 0 | 0 | + | **`psu`** | **564** | **0** | **0** | **0** | + + `psu` ≡ `(dist, ta, ea)` exactly (each is unique within the other), with 20 + households in every group. The `(dist, ea)` row is the instructive one: it is + the "make `v` a district composite" reflex, and it is *wrong here* — it still + merges EAs that share a district but sit in different TAs. + +2. **2013-14 is dispersal, and the GPS proves it.** 188 of 204 EAs have + households at different coordinates — the broken-key signature. But restrict + to households that did not move (`dist_to_IHS3location` ≤ 1 km) and **all 204 + EAs collapse to exactly one coordinate**; `LAT_DD_MOD` is the EA-level + displaced fix of the household's *current* location, not per-household jitter. + Median within-EA spread 145 km, max 775 km — every kilometre of it a tracked + mover. Corroborated: 588/4,000 households report a district different from the + one encoded in their own `ea_id`, sitting a median **68 km** from their IHS3 + location, against **0.03 km** for the 3,412 that match. + + This is the finding that made the difference between fixing an identifier and + fabricating one. The brief's premise — "at 75%, the key is broken" — is right + about 2004-05 and wrong about 2013-14, and the reason is arithmetic: with ~20 + households per EA and 13% of them district-movers, `1 − 0.87²⁰ ≈ 94%` of EAs + are *expected* to contain at least one mover. The high **cell** rate is a + property of the metric on a tracked panel, not evidence of a merged key. + +3. **The 2016-17 `Region` conflict was a spelling, not a collision.** EA + `30305580` is in both halves; the CS file writes `Southern`, the panel file + `South`. Same district, same real EA. Applying the `region` table at + extraction removes the cell. 1 → 0. + +4. **2016-17 had no GPS at all, and it was a merge key.** `df_main` builds + Cross_Sectional `i` as `cs_i(case_id)`; `df_geo` declared the raw `case_id`, + so the `how='outer'` join matched nothing — 12,447 orphan geo rows next to + 14,955 coordinate-less main rows, and **0 of 880 clusters with GPS**. The two + files are exactly 1:1 on `case_id` (12,447 = 12,447, none on either side + alone), so mirroring the transform makes the join exact: **779 of 880** + clusters now carry coordinates (the other 101 are IHPS panel EAs, for which + IHS4 publishes no geovariables). Honest cost: 7 of those 779 EAs have + households whose `lat_modified` differs within the EA — IHS4's displacement is + nearly but not exactly EA-constant — so `Latitude`/`Longitude` show 7 + contested cells where before there were none, *because before there was no + coordinate*. + + This is **not** a Site-4 cardinality fix and 2016-17 is not on PR #627's + cartesian list. It is a non-matching key, which produces orphans, not a + product. + +5. **The two real cartesians were fixed, measured, and then reverted.** 2010-11 + (196,083 rows from 12,271; 183,812 phantom) and 2019-20 (185,842 from 14,612; + 171,230 phantom) merge a household-grain geo file on the cluster key `v`. + Collapsing both to `merge_on: [i]` was implemented and measured: contested + cells **0 and 250 — identical either way**, confirming §4's invariance claim + by measurement rather than by argument. The change was then reverted at the + coordinator's direction so PR #627's guard retains these cells as evidence; + the one-line fix is recorded in each wave's `data_info.yml` next to the + declaration that causes it, and in `CONTENTS.org`. #627 owns the decision. + +6. **`Rural` needed nothing.** The pre-#602 inline `mapping:` blocks were both + live and inverted (`rural: 0` in one file, `RURAL: 1` in another, in the *same + wave*), but GH #602 already removed all five on `development`. Verified + against the pre-collapse frames: `Rural` arrives as the raw label and + `_enforce_canonical_spellings` normalises it. Nothing added. + +## §7 Measured effect (cold build, isolated `LSMS_DATA_DIR`) + +Contested cells = clusters where `nunique(dropna=True) > 1` for that attribute, +counted on the frame captured *inside* `_collapse_to_cluster_grain` before it +runs — the only place the evidence still exists. + +| wave | before | after | clusters | note | +|------|--------|-------|----------|------| +| 2004-05 | 164 / 330 (49.7%) | **0 / 1,692** | 110 → **564** | Region 66→0, District 79→0, Rural 19→0 | +| 2010-11 | 0 / 3,840 | 0 / 3,840 | 768 | already correct | +| 2013-14 | 765 / 1,020 (75.0%) | 765 / 1,020 | 204 | dispersal — §6.2 | +| 2016-17 | 78 / 4,405 | 89 / 4,400 | 880 | Region 1→**0**; Lat/Lon 1→7 *because GPS now exists*: 0 → **779** clusters with coordinates | +| 2019-20 | 250 / 4,095 (6.1%) | 250 / 4,095 | 819 | dispersal — panel half only | +| **total** | **1,257 / 13,690** | **1,104 / 15,047** | 2,782 → **3,235** | | + +(The brief's baseline was 1,255; the 2-cell difference is in 2004-05 and does not +bear on anything. Its 424,607 source rows are 412,160 here — the 12,447 removed +are 2016-17's orphan geo rows, and ~355k of what remains is the known cartesian.) + +At the API, what the 2004-05 fix restored: **20 → 26 districts**, and the +settlement strata from `{Rural}` — every one of the 110 mega-clusters came out +rural — back to `{Rural, Urban}`. + +Two of the five waves (2004-05, 2010-11) now emit **no `GrainCollapseWarning` at +all**; before, four did. + +## §8 Verification + +- `sample.v` ≡ `cluster_features.v` in **every** wave: **0** `cluster_features` + `(t, v)` pairs unknown to `sample`, **0** orphaned `sample` clusters. +- `tests/test_gh323_malawi_cluster_key.py` — 14 tests, all config-level; + exercises no core aggregation. **Negative control run**: 7 of the 14 fail + against a pristine `origin/development` config tree (exported with + `git archive`, cold data dir), including every load-bearing one. +- Malawi-relevant slice (`-k "malawi or Malawi or 323 or schema or sample or + cluster"`): **680 passed, 58 skipped, 4 xfailed, 1 failed**. +- The 1 failure is **pre-existing and untouched by this branch**: + `tests/test_gh323_explicit_reducers.py::test_core_does_not_dispatch_the_reducers` + greps `lsms_library/country.py` for the substring `collapse_to_cluster_grain` + and finds PR #617's private `_collapse_to_cluster_grain`. This branch is + config-only and does not touch `country.py`. Same failure is reported on PR + #634. + +## §9 Deferred (deliberately) + +- **Site 4 / PR #627**: 2010-11 and 2019-20's cartesian `df_geo` merges. §6.5. +- **The `region`/`district` extraction-time harmonisation is applied only where + it removed a measured conflict** (2016-17 `Region`). Doing it everywhere is + output-neutral and tidier, but it is a diff across five waves and two tables + for zero measured gain, so it is not in this PR. +- **2016-17's 101 panel clusters and 2019-20's 102 have no coordinates**, because + IHS4/IHS5 publish geovariables for the cross-sectional half only. Pre-existing, + unrelated to #323, recorded in `CONTENTS.org`. +- **IHPS-2016 mixes baseline and current geography in one file** (`district` + baseline, `reside` current — 0 vs 277 households off their EA's mode). That is + the source's inconsistency; documented, not papered over. + +--- +### Phase 3 — verification + +- `Malawi/2004-05/_/data_info.yml` `sample.v` / `cluster_features.v` = `psu` — + **OK (anchored on §3, §5)**: same key in both, as `_join_v_from_sample` + requires; verified 0 unknown pairs. +- `Malawi/2016-17/_/data_info.yml` `df_geo.idxvars.i` — **OK (anchored on §2)**: + reuses the existing `cs_i`, mirroring `df_main`; no new transform. +- `Malawi/2016-17/_/data_info.yml` `Region` `mappings:` — **OK (anchored on §2)**: + same `mappings: [table, from, to]` form the wave's `strata` already uses; the + `region` table is unchanged. +- 2013-14 / 2019-20 — **OK (anchored on §5, §6.2)**: no change is the decision, + and the decision is pinned by a test that fails if the evidence for it changes. +- No `aggregation:` key added, no reducer declared, no value blanked to ``, + no file under `lsms_library/*.py` touched — **OK (anchored on §3, D1)**. diff --git a/lsms_library/countries/Malawi/2004-05/_/data_info.yml b/lsms_library/countries/Malawi/2004-05/_/data_info.yml index 0dbc0f823..e24a96ca2 100644 --- a/lsms_library/countries/Malawi/2004-05/_/data_info.yml +++ b/lsms_library/countries/Malawi/2004-05/_/data_info.yml @@ -6,7 +6,23 @@ sample: idxvars: i: case_id myvars: - v: ea + # GH #323 SITE 2. `ea` is NOT a cluster id: it is the EA's sequence + # number WITHIN its Traditional Authority, so it takes only 110 + # distinct values for IHS2's 564 enumeration areas. Keyed on `ea`, + # ~5 unrelated EAs -- in different TAs, districts and regions -- + # merge into one `v`. `psu` is the fully qualified 8-digit EA code + # (region 1 + district 2 + TA 2 + EA 3) that the same file already + # carries: 564 distinct values, exactly 20 households each, and it + # is the SAME 8-digit keyspace the later IHS3/IHS4/IHS5 waves use + # for `ea_id` (a different census frame vintage, hence only 3 codes + # in common with 2010-11 -- the FORMAT matches, the frame does not). + # Verified: keyed on `psu`, zero households in a cluster disagree + # about the cluster's region / district / urban-rural status; keyed + # on `ea`, 164 of 330 such cells are contested. + # + # `v` must be declared identically here and in cluster_features + # below or _join_v_from_sample matches nothing. + v: psu weight: hhwght panel_weight: hhwght strata: @@ -40,7 +56,15 @@ cluster_features: file: sec_a.dta idxvars: i: case_id - v: ea + # GH #323 SITE 2 -- see the note on sample.v above. `ea` is the EA's + # sequence number within its Traditional Authority (110 values for + # 564 EAs); `psu` is the fully qualified 8-digit EA code. Keeping + # `ea` here merged ~5 real EAs per `v` and the household->cluster + # projection then served ONE arbitrary household's answer as the + # cluster's: 6 of Malawi's 26 districts vanished from + # cluster_features entirely and every one of the 110 surviving + # clusters came out `Rural` -- IHS2's whole urban stratum erased. + v: psu myvars: Region: region District: dist diff --git a/lsms_library/countries/Malawi/2010-11/_/data_info.yml b/lsms_library/countries/Malawi/2010-11/_/data_info.yml index f670d410e..578910820 100644 --- a/lsms_library/countries/Malawi/2010-11/_/data_info.yml +++ b/lsms_library/countries/Malawi/2010-11/_/data_info.yml @@ -129,6 +129,23 @@ cluster_features: # lookups for this wave. District: hh_a01 df_geo: + # GH #323 SITE 4 -- KNOWN CARTESIAN, DELIBERATELY NOT FIXED HERE. + # `householdgeovariables.dta` is HOUSEHOLD grain (12,271 rows, one per + # case_id, across 768 EAs), and this block keys it on `ea_id` and merges + # on the CLUSTER key `v`. `pd.merge` therefore pairs every household in + # df_main with every geo row sharing that `v`: 12,271 x ~16 = 196,083 + # rows, 183,812 of them phantom, for a table that collapses to 768 + # clusters. It costs 16x the time and memory and manufactures the + # duplicates Site 2 then destroys. + # + # It does NOT corrupt any value: `lat_modified` is constant within every + # ea_id, and a cartesian repeats values rather than inventing them, so + # the contested-cell count is provably unchanged by it (measured: 0 + # before and after collapsing the merge to household grain). The + # one-line fix is `idxvars: {i: case_id}` + `merge_on: [i]` -- the two + # files are exactly 1:1 on case_id, no rows on either side alone. Left + # in place so that PR #627's cardinality guard (core, Site 4) still has + # this cell as evidence; #627 owns the decision. file: Full_Sample/Geovariables/HH_level/householdgeovariables.dta idxvars: v: ea_id diff --git a/lsms_library/countries/Malawi/2016-17/_/data_info.yml b/lsms_library/countries/Malawi/2016-17/_/data_info.yml index 9a4691772..ccbdf1df3 100644 --- a/lsms_library/countries/Malawi/2016-17/_/data_info.yml +++ b/lsms_library/countries/Malawi/2016-17/_/data_info.yml @@ -60,7 +60,19 @@ cluster_features: - mapping: cs_i v: ea_id myvars: - Region: region + # GH #323 SITE 2. Harmonize the region label AT EXTRACTION, not + # only at API time. The Cross_Sectional file spells the southern + # region 'Southern' and the Panel file spells it 'South'; EA + # 30305580 is sampled in BOTH halves (a genuine shared EA, same + # district -- not a code collision), so the household->cluster + # projection saw two spellings of one region and recorded a + # conflict that does not exist. The auto-discovery `region` + # table already collapses them at API time, so applying it here + # is output-neutral -- it just stops the collapse from seeing a + # spelling difference as a disagreement about the cluster. + Region: + - region + - mappings: ['region', 'Alternate Spelling', 'Preferred Label'] District: district # Raw `reside` is already a declared spelling of the canonical # Rural/Urban vocabulary in every Malawi wave ('Rural'/'Urban', @@ -78,7 +90,18 @@ cluster_features: # file in this wave and end up with NaN lat/lon -- acceptable. file: Cross_Sectional/householdgeovariablesihs4.dta idxvars: - i: case_id + # `i` MUST be built exactly as df_main builds it. df_main runs + # case_id through cs_i ('cs-17-' + format_id) to keep the + # Cross_Sectional ids apart from the Panel's y3_hhid; this block + # declared the RAW case_id, so the merge key never matched and + # the `how='outer'` join produced 12,447 orphan geo rows next to + # 14,955 main rows with no coordinates. Result: 0 of 880 + # clusters had GPS. The two files are exactly 1:1 on case_id + # (12,447 = 12,447, no rows on either side alone), so mirroring + # the transform makes the join exact. + i: + - case_id + - mapping: cs_i myvars: Latitude: lat_modified Longitude: lon_modified diff --git a/lsms_library/countries/Malawi/2019-20/_/data_info.yml b/lsms_library/countries/Malawi/2019-20/_/data_info.yml index 6e2279d89..40d8af22f 100644 --- a/lsms_library/countries/Malawi/2019-20/_/data_info.yml +++ b/lsms_library/countries/Malawi/2019-20/_/data_info.yml @@ -73,6 +73,24 @@ cluster_features: # sign-flipping the indicator between waves. GH #602. Rural: reside df_geo: + # GH #323 SITE 4 -- KNOWN CARTESIAN, DELIBERATELY NOT FIXED HERE. + # `householdgeovariables_ihs5.dta` is HOUSEHOLD grain (11,434 rows, one + # per case_id, across 717 EAs), and this block keys it on `ea_id` and + # merges on the CLUSTER key `v`: 14,612 households x 11,434 geo rows + # paired within `v` = 185,842 rows, 171,230 of them phantom, for a table + # that collapses to 819 clusters. + # + # It does NOT corrupt any value -- a cartesian repeats values rather + # than inventing them, and the contested-cell count is provably + # unchanged by it (measured: 250 before and after collapsing the merge + # to household grain). The one-line fix is `idxvars: {i: [case_id, + # {function: cs_i}]}` + `merge_on: [i]`; the two files are exactly 1:1 + # on case_id. Left in place so PR #627's cardinality guard (core, + # Site 4) still has this cell as evidence; #627 owns the decision. + # + # Unrelated to the cardinality: the 102 IHPS Panel EAs share NO ea_id + # with the geo file, so those clusters have NaN coordinates either way + # -- IHS5 publishes no geovariables for the panel half. file: Cross_Sectional/householdgeovariables_ihs5.dta idxvars: v: ea_id diff --git a/lsms_library/countries/Malawi/_/CONTENTS.org b/lsms_library/countries/Malawi/_/CONTENTS.org index db2de88d5..82a14f71a 100644 --- a/lsms_library/countries/Malawi/_/CONTENTS.org +++ b/lsms_library/countries/Malawi/_/CONTENTS.org @@ -465,9 +465,139 @@ District names vary slightly across waves (e.g., "Blanytyre" vs =strata= table in =categorical_mapping.org=. ** Cluster (PSU) -- 2004-05: =ea= (numeric, 110 unique EAs) +- 2004-05: =psu= (string, 564 unique EAs) -- *was* =ea=; see below - 2010-11 onward: =ea_id= (string, 200--800 unique EAs per wave) +Every wave's =v= is the same object in the same format: the *fully qualified +8-digit Malawi enumeration-area code*, =region(1) + district(2) + TA(2) + +EA(3)= -- e.g. =10101016= is EA 016 of TA 01 of district 01 (Chitipa) of +region 1 (North). + +*** GH #323: 2004-05's cluster key was not a cluster key + +Until this was fixed, 2004-05 declared =v: ea=. =ea= is *the EA's sequence +number within its Traditional Authority*, so it takes only *110 distinct +values for IHS2's 564 enumeration areas*. Keyed on =ea=, roughly five +unrelated EAs -- in different TAs, districts and regions -- merged into one +=v=, and the household -> cluster projection in =Wave.cluster_features= then +resolved the disagreement with =groupby().first()=: one arbitrary household's +answer, served as the cluster's. + +What that destroyed, measured cold: + +| quantity | =v: ea= | =v: psu= | +|-----------------------------------------------------+---------+----------| +| clusters | 110 | 564 | +| districts present in =cluster_features= | 20 | 26 | +| settlement strata present in =cluster_features= | 1 (all =Rural=) | 2 | +| contested cluster-attribute cells (of 330 / 1,692) | 164 | *0* | + +The urban stratum did not merely lose rows -- it disappeared. All 110 +surviving clusters came out =Rural=, because =first()= happened to reach a +rural household in every merged group. Six of Malawi's 26 districts vanished +from =cluster_features= entirely. Both facts were invisible at the API: the +table looked complete, it was just wrong. + +=psu= is the fully qualified code, already present in =sec_a.dta=: 564 values, +*exactly 20 households each* (the IHS2 design), and *zero* households in a +cluster disagreeing about their cluster's region, district or urban/rural +status. It is declared identically in =sample= and in =cluster_features=, +because =_join_v_from_sample= joins the two through =v= and they cannot be +allowed to drift. + +Note the *frame vintage*: 2004-05 shares only 3 codes with 2010-11. IHS2 is +drawn from the 1998 census frame and IHS3+ from the 2008 frame, so the code +*format* is common across all five waves but the code *sets* are not. A +cross-wave join on =v= is meaningful from 2010-11 onward and not before. + +*** GH #323: what remains contested, and why it is NOT re-keyed + +After the fix, three waves still have clusters whose households disagree. In +every case the measurement says the identifier is *right* and the households +have *moved* -- these are tracked panels, and re-keying on the current district +would fracture real EAs, desynchronise =v= from =sample=, and sever the panel's +link to its own baseline. + +| wave | contested cells | of | what they are | +|---------+-----------------+-------+----------------------------------------------| +| 2004-05 | *0* | 1,692 | -- | +| 2010-11 | *0* | 3,840 | -- | +| 2013-14 | 765 | 1,020 | IHPS tracked movers (see below) | +| 2016-17 | 75 | 4,400 | IHPS-2016 =reside= is current, not baseline | +| 2019-20 | 250 | 4,095 | IHPS tracked movers | + +**** 2013-14 (IHPS): the GPS is the proof +=ea_id= here is the *IHS3 baseline EA* -- 204 of IHS3's 768, and =case_id= is +literally =ea_id= + household number. The survey tracks movers and split-off +households wherever they go (4,000 interviewed households from 3,104 baseline +households), and the file's =region= / =district= / =reside= / geovariables all +describe the household's *current* location. + +188 of 204 EAs have households reporting different coordinates, which is +exactly the broken-key signature. It is not one. Restrict to households that +did *not* move (=dist_to_IHS3location= <= 1 km) and *every one of the 204 EAs +collapses to exactly one coordinate* -- =LAT_DD_MOD= is the EA-level displaced +fix of the household's current location, not per-household jitter. All of the +within-EA spread (median 145 km, max 775 km) is dispersal. + +Corroborating: 588 of 4,000 households report a district different from the one +encoded in their own =ea_id=, and those 588 sit a *median 68 km* from their +IHS3 location against *0.03 km* for the 3,412 that match. + +**** 2016-17 (IHS4 + IHPS4) +The 75 remaining cells are all =Rural=, all in the panel half. IHPS-2016 +records =district= / =region= at *baseline* (0 of 2,508 households sit outside +their EA's modal district) but =reside= *currently* -- 277 households differ +from their EA's modal =reside= and 246 of those are flagged =mover=. So one +file mixes baseline and current geography; that is the survey's inconsistency, +not ours. + +EA =30305580= is sampled in *both* the Cross_Sectional and the Panel halves. +That is a genuine shared EA (same district), not a code collision -- the two +halves spelled its region =Southern= and =South=, which used to read as a +disagreement. The =region= table is now applied at extraction, so it does not. + +**** 2019-20 (IHS5 + IHPS5) +All 250 cells are in the panel half. Unlike 2016-17, IHPS-2019 records +=district= / =region= *currently*: 747 of 3,178 households are outside their +EA's modal district, and 652 of those are more than 1 km from their IHS3 +location. The 102 panel EAs share no =ea_id= with the 717 cross-sectional +ones, so there is no cross-half collision here at all. + +*** GH #323: 2016-17 had no GPS, and the reason was a merge key +=cluster_features= builds =i= for the Cross_Sectional half by running =case_id= +through =cs_i= (='cs-17-'= + =format_id=), to keep it apart from the Panel's +=y3_hhid=. The =df_geo= block declared the *raw* =case_id=, so its merge key +never matched anything: the =how='outer'= join produced 12,447 orphan geo rows +alongside 14,955 main rows with no coordinates, and *0 of 880 clusters had +GPS*. =df_geo= now builds =i= exactly as =df_main= does; the two files are +1:1 on =case_id= (12,447 = 12,447), so the join is exact and *779 of 880* +clusters carry coordinates. The remaining 101 are the IHPS panel EAs, for +which IHS4 publishes no geovariables file. + +Cost of that fix, stated plainly: 7 of the 779 EAs have households whose IHS4 +=lat_modified= differs within the EA (IHS4's displacement is *nearly* but not +exactly EA-constant), so =Latitude= / =Longitude= now show 7 contested cells +where before there were none -- because before there was no coordinate at all. + +*** Known, deliberately unfixed: two cartesian =df_geo= merges (Site 4, PR #627) +2010-11 and 2019-20 key a *household-grain* geovariables file on =ea_id= and +merge it on the *cluster* key =v=, so =pd.merge= pairs every household with +every geo row sharing that =v=: + +| wave | households | geo rows | merged rows | phantom | +|---------+------------+----------+-------------+---------| +| 2010-11 | 12,271 | 12,271 | 196,083 | 183,812 | +| 2019-20 | 14,612 | 11,434 | 185,842 | 171,230 | + +A cartesian *repeats* values, it does not invent them, so it corrupts nothing +and changes no contested-cell count -- verified by measuring both waves with +the merge collapsed to household grain (=merge_on: [i]=, the two files are 1:1 +on =case_id=): 0 and 250 contested cells respectively, identical either way. +It does cost roughly 16x the build time and memory. The one-line config fix is +noted in each wave's =data_info.yml= but *not applied here*, so that PR #627's +cardinality guard keeps these cells as evidence. #627 owns the decision. + ** Weights | Wave | Cross-section weight | Panel weight | Source file | |---------+----------------------+--------------------+--------------------------------------------| diff --git a/tests/test_gh323_malawi_cluster_key.py b/tests/test_gh323_malawi_cluster_key.py new file mode 100644 index 000000000..99abad8c9 --- /dev/null +++ b/tests/test_gh323_malawi_cluster_key.py @@ -0,0 +1,299 @@ +"""Malawi cluster key (GH #323, Site 2). + +Malawi's `cluster_features` is declared at `(t, v)` but every wave extracts it +from the household cover page, so it is projected from household grain onto the +cluster. Where the households of one `v` DISAGREE about the cluster's own +attributes, core resolves it with `groupby().first()` -- one arbitrary +household's answer, served as the cluster's. These tests pin the CONFIG that +makes `v` a real cluster id. They exercise no core aggregation (D1: core does +not aggregate). + +Two different diseases were measured, and only one of them is a broken key: + +* **2004-05 (IHS2) -- BROKEN KEY.** `ea` is the EA's sequence number *within + its Traditional Authority*: 110 distinct values for 564 enumeration areas, so + ~5 unrelated EAs merged into one `v`. Fixed by keying on `psu`, the fully + qualified 8-digit EA code (region + district + TA + EA) that the same file + already carries. 164 of 330 contested cells -> 0. + +* **2013-14 / 2019-20 (IHPS panel halves) -- NOT a broken key.** `ea_id` is the + IHS3 baseline EA and the survey TRACKS movers, so a cluster's households + genuinely live in different districts. The decisive evidence is the GPS: in + 2013-14 the households of an EA that did NOT move share EXACTLY ONE + coordinate, in all 204 EAs. Every kilometre of within-EA spread is a tracked + mover. Re-keying on the current district would fracture real EAs, desynchronise + `v` from `sample`, and destroy the panel's link to its own baseline. So the + residual contested cells are reported, not forced to zero. + +Data-dependent: skips cleanly when the Malawi source files are unavailable. +""" +import re + +import pandas as pd +import pytest +import yaml + +import lsms_library as ll +from lsms_library.paths import countries_root + +WAVES = ["2004-05", "2010-11", "2013-14", "2016-17", "2019-20"] + +# The fully qualified Malawi EA code: region(1) + district(2) + TA(2) + EA(3). +EA_CODE = re.compile(r"^\d{8}$") + + +@pytest.fixture(scope="module") +def mw(): + return ll.Country("Malawi") + + +def _build(mw, table): + try: + df = getattr(mw, table)() + except Exception as exc: # noqa: BLE001 - any build/data failure -> skip + pytest.skip(f"Malawi {table} unavailable: {exc}") + if df is None or df.empty: + pytest.skip(f"Malawi {table} empty") + return df + + +@pytest.fixture(scope="module") +def sample(mw): + return _build(mw, "sample") + + +@pytest.fixture(scope="module") +def cluster_features(mw): + return _build(mw, "cluster_features") + + +def _wave_config(wave): + path = countries_root() / "Malawi" / wave / "_" / "data_info.yml" + if not path.exists(): + pytest.skip(f"Malawi {wave} data_info.yml missing") + with open(path) as f: + return yaml.safe_load(f) + + +# --------------------------------------------------------------------------- +# 1. 2004-05: the broken key. +# --------------------------------------------------------------------------- + +def test_2004_05_v_is_the_fully_qualified_ea_code_not_the_within_TA_sequence(sample): + """IHS2 drew 564 EAs of 20 households each. `ea` collapses them to 110.""" + s = sample.reset_index() + v = s.loc[s["t"].astype(str) == "2004-05", "v"].dropna().astype(str) + assert v.nunique() == 564, ( + f"2004-05 should have 564 clusters (IHS2's EA count), got {v.nunique()}. " + "110 means `v` is still `ea`, the EA sequence number within its " + "Traditional Authority, which merges ~5 real EAs per cluster." + ) + assert v.map(lambda x: bool(EA_CODE.match(x))).all(), ( + "2004-05 `v` must be the 8-digit region+district+TA+EA code" + ) + + +def test_2004_05_cluster_features_keeps_every_district_and_the_urban_stratum( + cluster_features, +): + """What the broken key actually destroyed, at the API. + + Keyed on `ea`, `groupby().first()` served one household's answer for each of + 110 mega-clusters: 6 of Malawi's 26 districts disappeared from + `cluster_features` outright, and every surviving cluster came out `Rural` -- + IHS2's entire urban stratum erased from the cluster table. + """ + cf = cluster_features.reset_index() + w = cf[cf["t"].astype(str) == "2004-05"] + assert len(w) == 564, f"expected 564 clusters, got {len(w)}" + assert w["District"].nunique() == 26, ( + f"2004-05 covers 26 districts; cluster_features shows " + f"{w['District'].nunique()}" + ) + assert set(w["Rural"].dropna().astype(str)) == {"Rural", "Urban"}, ( + "2004-05 cluster_features must retain both settlement strata; " + f"got {sorted(set(w['Rural'].dropna().astype(str)))}" + ) + + +def test_2004_05_source_shows_ea_is_not_a_cluster_id(mw): + """The evidence, at source: `ea` merges districts, `psu` does not. + + This is the measurement that decided the key. It is pinned so that a future + reader does not have to take the diagnosis on trust, and so that a change of + source file cannot silently invalidate it. + """ + from lsms_library.local_tools import get_dataframe + + path = countries_root() / "Malawi" / "2004-05" / "Data" / "sec_a.dta" + try: + df = get_dataframe(str(path)) + except Exception as exc: # noqa: BLE001 + pytest.skip(f"Malawi 2004-05 sec_a.dta unavailable: {exc}") + + assert df["ea"].nunique() == 110 + assert df["psu"].nunique() == 564 + # exactly 20 households per real EA -- the IHS2 design + assert set(df.groupby("psu").size().unique()) == {20} + + def contested(key, cols): + g = df.assign(**{key: df[key].astype(str)}).groupby(key, observed=True) + return {c: int((g[c].nunique(dropna=True) > 1).sum()) for c in cols} + + cols = ["region", "dist", "reside"] + assert contested("psu", cols) == {"region": 0, "dist": 0, "reside": 0}, ( + "psu must be a real cluster id: no household disagrees about its cluster" + ) + bad = contested("ea", cols) + assert sum(bad.values()) > 100, ( + f"`ea` is expected to be badly contested (measured 164 cells); got {bad}" + ) + + +# --------------------------------------------------------------------------- +# 2. The invariant that makes the join work at all. +# --------------------------------------------------------------------------- + +@pytest.mark.parametrize("wave", WAVES) +def test_sample_v_and_cluster_features_v_are_the_same_key( + wave, sample, cluster_features +): + """`_join_v_from_sample` joins `cluster_features` onto every household table + through `sample.v`. If the two `v` are built from different columns the + join silently matches nothing, so this must hold in EVERY wave -- it is the + reason the 2004-05 change had to be made in `sample` and `cluster_features` + together.""" + s = sample.reset_index() + cf = cluster_features.reset_index() + sv = set(s.loc[s["t"].astype(str) == wave, "v"].dropna().astype(str)) + cv = set(cf.loc[cf["t"].astype(str) == wave, "v"].dropna().astype(str)) + if not sv and not cv: + pytest.skip(f"{wave} absent from this build") + assert cv - sv == set(), ( + f"{wave}: {len(cv - sv)} cluster_features clusters unknown to sample: " + f"{sorted(cv - sv)[:5]}" + ) + assert sv - cv == set(), ( + f"{wave}: {len(sv - cv)} sample clusters have no cluster_features row: " + f"{sorted(sv - cv)[:5]}" + ) + + +def test_every_wave_uses_the_same_eight_digit_ea_keyspace(sample): + """One `v` vocabulary across the whole country. + + IHS2's frame is a different census vintage from IHS3+ (only 3 codes in + common), so the SETS differ -- but the FORMAT must not, or `v` stops being + comparable and a cross-wave join silently produces nothing. + """ + s = sample.reset_index() + for wave in WAVES: + v = s.loc[s["t"].astype(str) == wave, "v"].dropna().astype(str) + if v.empty: + continue + bad = sorted(set(v[~v.map(lambda x: bool(EA_CODE.match(x)))]))[:5] + assert not bad, f"{wave}: non-canonical cluster ids {bad}" + + +# --------------------------------------------------------------------------- +# 3. The df_geo merges: no cartesian product, and the key actually matches. +# --------------------------------------------------------------------------- + +@pytest.mark.parametrize("wave", ["2010-11", "2019-20"]) +def test_the_known_cartesian_geo_merges_are_documented_where_they_live(wave): + """2010-11 and 2019-20 merge a HOUSEHOLD-grain geo file on the CLUSTER key. + + That pairs every household with every geo row sharing its `v`: 196,083 rows + from 12,271 (183,812 phantom) and 185,842 from 14,612 (171,230 phantom). + It is Site 4, it is owned centrally by PR #627's cardinality guard, and it + is deliberately NOT fixed here -- but it must not be silent either. A + cartesian repeats values rather than inventing them, so it changes no + contested-cell count; see the paired test below, which is the load-bearing + claim. + """ + cfg = _wave_config(wave)["cluster_features"] + assert cfg.get("merge_on") == ["v"], ( + f"{wave}: if this now merges on `i` the cartesian has been fixed -- " + "update the note in data_info.yml and tell PR #627 it lost an example" + ) + src = (countries_root() / "Malawi" / wave / "_" / "data_info.yml").read_text() + assert "SITE 4" in src and "CARTESIAN" in src, ( + f"{wave}: the known cartesian df_geo merge must stay documented in " + "data_info.yml, next to the declaration that causes it" + ) + + +@pytest.mark.parametrize("wave", ["2016-17"]) +def test_geo_subframe_builds_i_exactly_as_the_main_frame_does(wave): + """Both halves of IHS4/IHS5 live in one table, so `i` for the + Cross_Sectional half is run through `cs_i` to keep it apart from the Panel's + y{3,4}_hhid. The geo block declared the RAW case_id, so its merge key never + matched: 2016-17 ended up with GPS for 0 of its 880 clusters, plus 12,447 + orphan geo rows manufactured by the outer join. The two blocks must build + `i` identically or they drift apart again.""" + cfg = _wave_config(wave)["cluster_features"] + main_i = cfg["df_main"]["idxvars"].get("i") + geo_i = cfg["df_geo"]["idxvars"].get("i") + assert geo_i is not None, f"{wave}: df_geo declares no `i` to merge on" + assert geo_i == main_i, ( + f"{wave}: df_geo `i` ({geo_i!r}) must be built exactly like df_main's " + f"({main_i!r})" + ) + + +def test_2016_17_clusters_actually_have_coordinates(cluster_features): + """The consequence of the key fix above, measured at the API: 0 -> 779.""" + cf = cluster_features.reset_index() + w = cf[cf["t"].astype(str) == "2016-17"] + if w.empty: + pytest.skip("2016-17 absent from this build") + have = int(w["Latitude"].notna().sum()) + assert have >= 700, ( + f"2016-17 should carry GPS for its 779 Cross_Sectional clusters " + f"(the 101 Panel clusters have no geovariables file); got {have}" + ) + + +# --------------------------------------------------------------------------- +# 4. The residual, and why it is NOT re-keyed. +# --------------------------------------------------------------------------- + +def test_2013_14_within_cluster_spread_is_tracked_movers_not_a_broken_key(): + """The decisive measurement, pinned. + + 188 of 204 EAs have households reporting different coordinates, which looks + exactly like the broken-key signature. It is not: restrict to households + that did NOT move (`dist_to_IHS3location` <= 1 km) and every one of the 204 + EAs collapses to EXACTLY ONE coordinate. `LAT_DD_MOD` is the EA-level + displaced fix of the household's CURRENT location, so all of the spread is + panel dispersal. `ea_id` is a real cluster id and must be left alone. + + If this ever fails, the diagnosis behind leaving 2013-14 / 2019-20 unre-keyed + has changed and the decision must be revisited. + """ + from lsms_library.local_tools import get_dataframe + + root = countries_root() / "Malawi" / "2013-14" / "Data" + try: + cover = get_dataframe(str(root / "HH_MOD_A_FILT_13.dta")) + geo = get_dataframe(str(root / "HouseholdGeovariables_IHPS_13.dta")) + except Exception as exc: # noqa: BLE001 + pytest.skip(f"Malawi 2013-14 sources unavailable: {exc}") + + m = cover[["y2_hhid", "ea_id", "dist_to_IHS3location"]].merge( + geo[["y2_hhid", "LAT_DD_MOD", "LON_DD_MOD"]], on="y2_hhid", how="inner" + ).dropna(subset=["LAT_DD_MOD"]) + + stayers = m[m["dist_to_IHS3location"].fillna(0) <= 1] + per_ea = stayers.groupby("ea_id")[["LAT_DD_MOD", "LON_DD_MOD"]].apply( + lambda g: len(g.drop_duplicates()) + ) + assert per_ea.max() == 1, ( + "households that did not move must share their EA's single displaced " + f"coordinate; {int((per_ea > 1).sum())} EAs do not" + ) + # and the movers really are the whole story + movers = m[m["dist_to_IHS3location"].fillna(0) > 1] + assert len(movers) > 1000, ( + f"expected ~1,340 tracked movers in IHPS 2013; got {len(movers)}" + ) From 38e6d28e40823a11408be9000324ec496585e8a5 Mon Sep 17 00:00:00 2001 From: Ethan Ligon Date: Tue, 21 Jul 2026 20:32:49 -0700 Subject: [PATCH 2/3] fix(#323/#627): Malawi's and Guinea-Bissau's cartesian `dfs:` merges MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #627's 40-country census found 8 many-to-many `dfs:` merges fabricating 4,907,774 phantom rows. Mali's 4.3M went in PR #641. These are the three nobody owned: Malawi 2010-11 12,271 x 12,271 -> 196,083 rows (183,812 phantom) Malawi 2019-20 14,612 x 11,434 -> 185,842 rows (171,230 phantom) Guinea-Bissau 2018-19 5,351 x 450 -> 5,410 rows ( 59 phantom) All three now emit 0 phantom rows, measured cold in an isolated data root with #627's `_cartesian_keys` detector on PYTHONPATH. They are TWO DIFFERENT SHAPES and take two different cures. MALAWI -- a household-grain frame keyed on the cluster. Both sub-frames are one row per household, merged on `v`, so pd.merge paired every household of an EA with every other, and `_normalize_dataframe_index` then mopped the wreck up with groupby().first() down to 768 / 819 clusters -- so the table read clean at every point anyone ever looked. The geo files carry `case_id`, so the honest key was there all along: `idxvars: {i: case_id}` + `merge_on: [i]` (2019-20 runs it through `cs_i`, exactly as its df_main does). 196,083 -> 12,271 and 185,842 -> 14,612 wave rows. PR #639's author investigated these two cells, implemented the same one-line fix, measured it, then deliberately REVERTED it so #627 would keep its evidence, leaving the diagnosis in the YAML. Every claim in that note was re-verified here from source rather than taken on trust, and all of it holds: the file pairs are exactly 1:1 on case_id (12,271 = 12,271 and 11,434 = 11,434, zero rows on either side alone, no duplicate ids); lat/lon are constant within all 768 and all 717 EAs; and df_main.ea_id == df_geo.ea_id for every shared household. This branch is stacked on #639 and replaces its two "deliberately not fixed" comments. GUINEA-BISSAU -- not that shape, and it does not take that cure. `grappe_gps_gnb2018.dta` ships 450 rows for 445 grappes: five appear twice as VERBATIM duplicate records, same lat/lon/accuracy/altitude/GPS timestamp (the companion community cover s00a_co carries the identical 450/445 split, so the duplication happened once, on the community side of the export). There is no household id to re-key on -- the GPS file is genuinely grappe-grain and carries nothing finer, and both copies of every duplicate agree on `vague` -- and no single-file cure, because the cover page has no GPS columns at all. So this one takes #627's OTHER named remedy, reduce the sub-frame to the merge-key grain BEFORE the merge, via a `df_geo` hook in the wave's mapping.py. 5,410 -> 5,351 rows. Two things about that hook. It is dispatched BY NAME off the sub-frame key (`Wave.grab_data` resolves a sub-frame's df_edit with `column_mapping(, ...)`) -- real current behaviour on both `development` and #627's branch, but undocumented, with no prior art anywhere in the corpus, so it is documented in three places and pinned by a test. And it uses drop_duplicates() over EVERY column, key and payload alike: that is de-duplication, not aggregation (D1), and it removes a row only when an identical row already exists -- so two GPS fixes that actually DISAGREE would both survive and re-trip #627's guard rather than being silently reduced to one. `groupby().first()` or `keep='first'` would hide exactly that. VALUES DO NOT MOVE. The country-level `cluster_features` frame is bit-for-bit identical before and after for both countries -- DataFrame.equals True, index equal, dtypes equal (Malawi 3,235 x 5 over all five waves; Guinea-Bissau 450 x 4). Malawi's published lat/lon is the EA's displaced fix stamped on every household, so handing a household its own row returns the coordinate the `v`-merge was already returning it. One honest wrinkle, at HOUSEHOLD grain only: 8 IHS5 geo rows have a null coordinate while their 15 EA siblings carry the EA's fix, so those 8 households now get their own NULL instead of borrowing a sibling's value. The returned (t, v) table is unchanged -- .first() skips NA -- and the household-grain intermediate now tells the truth about which rows the survey actually geolocated. Tests assert on `grab_data('cluster_features')`, NOT `Wave.cluster_features()`: the latter runs the site-2 projection onto (t, v) first and returns 768 / 819 rows with or without the cartesian, so a test there is vacuous. Negative control run with the pre-fix configs restored in a fresh isolated data root: 6 failed, 10 passed -- the three cartesian tests report 5410 / 196083 / 185842, the three structural tests fail, and the ten source-invariant and cluster-count tests pass WITH THE BUG FULLY PRESENT, which is the blindness the module docstring warns about. After: 16 passed. ALSO IN HERE: GH #637's three `groupby().first()` sites in Malawi/_/malawi.py, reviewed for KEY SOUNDNESS. Same method (patch DataFrameGroupBy.first in-process, drive the wave scripts with runpy -- all three tables are `materialize: make`, so an in-process patch cannot see them through the subprocess; that is what made an earlier probe of a site like this report a meaningless "0"). plot_features_for_wave 0 groups >1 row on a non-null key food_coping_for_wave 0 groups >1 row months_food_inadequate_for_wave 0 groups >1 row No composite is ever built at any of the three, because no group ever has more than one row to build one from. For the two Module-H sites rows in == groups out EXACTLY (61,355 / 19,994 / 62,232 / 57,157 and 12,271 / 4,000 / 12,447 / 11,434, the latter being each wave's Module H household count). plot_features' 4 apparent duplicate groups consist ENTIRELY of null-plot_id rows. No `.first(skipna=False)` anywhere -- it would change nothing, and the repo already rejected that approach. All three of #637's traps were checked, and they need two different instruments. The exact equality above disposes of the first two: "they're only exact duplicates" is not reassurance -- but here there are no duplicate groups AT ALL, which is strictly stronger -- and invariance-by-missingness does not apply, since the household id is non-null in every row and either a shared id or a format_id collision would surface right there as a duplicate group (confirmed at source: case_id / y2_hhid are unique in Module H, format_id merges none). Trap 3 -- A BROKEN KEY CAN PRODUCE ZERO DUPLICATES (the Tanzania shocks inversion) -- defeats that instrument entirely, so it was checked separately with a PER-WAVE overlap of each table's `i` against the roster's: 100% for all three tables in all four waves, per wave rather than in aggregate, matching literally (101011000014 on both sides) and with no row-count inflation relative to source households x strategies. Worth running rather than waving away: 2019-20 has exactly the shape of a namespace split -- cs_i ('cs-19-' + format_id) in data_info.yml for sample / household_roster / cluster_features vs i_prefix='' in all three wave scripts -- but the prefix survives to the API on NEITHER side (0 of 14,612 ids carry it, in any table), so the two agree. Now pinned per wave, with household counts, by test_i_lives_in_the_same_namespace_as_the_roster. What the audit did find is a silent deletion: groupby() defaults to dropna=True, so 468 plot_features rows with a NULL plot_id (275 / 158 / 27+6 / 0+2) were being deleted without a word -- #323 §3b's delete-and-report, in a country script rather than in core. They are empty Module C roster stubs, not plots: 0 of all 468 carries a non-null ag_c04a / ag_c04b / ag_c04c. Dropping them is right; it should not have been silent. It is now an explicit dropna(subset=['plot_id']) before the groupby, and it is a provable no-op -- plot_features, food_coping and months_food_inadequate are each bit-for-bit identical before and after (68,339 x 10 / 200,738 x 1 / 40,152 x 2, DataFrame.equals True). Config/script only; nothing under lsms_library/*.py outside countries/{Malawi,Guinea-Bissau}/**. No `aggregation:` key. Ledger: .coder/ledger/323-malawi-gb-cartesian.md Refs #323, #627, #637. Co-Authored-By: Claude Opus 4.8 (1M context) --- .coder/ledger/323-malawi-gb-cartesian.md | 153 +++++++++ .../Guinea-Bissau/2018-19/_/data_info.yml | 25 ++ .../Guinea-Bissau/2018-19/_/mapping.py | 55 +++ .../countries/Guinea-Bissau/_/CONTENTS.org | 51 +++ .../countries/Malawi/2010-11/_/data_info.yml | 45 ++- .../countries/Malawi/2019-20/_/data_info.yml | 53 ++- lsms_library/countries/Malawi/_/CONTENTS.org | 179 +++++++++- lsms_library/countries/Malawi/_/malawi.py | 60 +++- tests/test_gh323_malawi_cluster_key.py | 46 ++- tests/test_gh323_malawi_gb_cartesian.py | 323 ++++++++++++++++++ tests/test_gh637_malawi_first_sites.py | 314 +++++++++++++++++ 11 files changed, 1221 insertions(+), 83 deletions(-) create mode 100644 .coder/ledger/323-malawi-gb-cartesian.md create mode 100644 tests/test_gh323_malawi_gb_cartesian.py create mode 100644 tests/test_gh637_malawi_first_sites.py diff --git a/.coder/ledger/323-malawi-gb-cartesian.md b/.coder/ledger/323-malawi-gb-cartesian.md new file mode 100644 index 000000000..74a2caeb2 --- /dev/null +++ b/.coder/ledger/323-malawi-gb-cartesian.md @@ -0,0 +1,153 @@ +# Prior-Art Ledger — GH #323 site 4 / #627 (Malawi + Guinea-Bissau cartesians) and #637 (Malawi `.first()` sites) + +> Per-task ledger. Inherits the repo §0 baseline in `STANDING.md`; cites +> `CLAUDE.md` and `lsms_library/data_info.yml` rather than re-copying them. + +**Search tier used:** ripgrep + git floor. gitnexus not consulted — the config +half touches no symbol, and the `malawi.py` half is a comment + one `dropna` +inside a function whose only callers are the four wave scripts that import it +by name (enumerated by grep). + +## §1 Task, restated + +Three of the eight cartesian `dfs:` cells from PR #627's 40-country census were +unowned after Mali's went in PR #641: + +| cell | L x R | merged | phantom | +|---|---|---|---| +| Malawi 2010-11 | 12,271 x 12,271 | 196,083 | 183,812 | +| Malawi 2019-20 | 14,612 x 11,434 | 185,842 | 171,230 | +| Guinea-Bissau 2018-19 | 5,351 x 450 | 5,410 | 59 | + +Plus: review the three `groupby().first()` sites in `Malawi/_/malawi.py` for +KEY SOUNDNESS (#637). Config/script only, no `aggregation:` key, fix the merge +never the aftermath. + +## §2 Existing machinery (this task's area) + +| symbol | path | what it does | tested? | reuse / extend / new | +|---|---|---|---|---| +| `Wave._cartesian_keys` / `_merge_subframes` | `origin/fix/323-site4-dfs-merge` `country.py:909/945` | exact many-to-many detector + phantom count | 16 tests on that branch | **measuring instrument only**; core untouched | +| `Wave.grab_data` `dfs:` block | `country.py:~1112-1215` | outer-merges sub-frames on `merge_on ∪ {t}` | yes | untouched | +| sub-frame `df_edit` dispatch | `country.py:802` + `:997` — `column_mapping(, ...)` then `mapping_info.pop('df_edit')` | runs a country/wave-module function named after the SUB-FRAME on that sub-frame, **before** the merge | no direct test before this PR | **used** for Guinea-Bissau; now pinned by a test | +| `_collapse_to_cluster_grain` (site 2) | `country.py:~4490` | projects household-grain `cluster_features` onto `(t, v)` | yes | not touched — it is why `Wave.cluster_features()` is blind here | +| PR #641 `Mali/2021-22` | `Mali/2021-22/_/data_info.yml` | deleted the `dfs:` block for a single-file extraction | yes | **template**; not applicable to either country here (see §5) | +| PR #639 `fix/323-malawi-config` | `Malawi/{2004-05,2016-17}/_/data_info.yml` | fixed 2004-05's cluster key and 2016-17's `df_geo` merge key | yes | **this branch is stacked on it**; its two "deliberately not fixed" comments are replaced | +| `tests/conftest.py::requires_s3` | `tests/conftest.py:71` | data-free-CI skip marker | yes | reused | + +## §3 Definitions & conventions in force + +- **D1, fix the merge, never reduce afterwards** — `CLAUDE.md` "Grain Collapse"; + `SkunkWorks/grain_aggregation_policy.org` §3a. `aggregation:` is dead config. +- **`dfs:` merges exist to be collapsed, not grown** — `CLAUDE.md` "Gotchas with + Teeth": *"Existing `dfs:` merges are grandfathered but should be collapsed + when touched"*. +- **The published cluster GPS is a displaced cluster fix stamped on every + household**, not household GPS — `CLAUDE.md` site-2 note (GH #161). Verified + here for both Malawi waves (0 of 768 / 0 of 717 EAs carry two coordinates). +- **`.first()` skips NA per column**, so a conflicting group yields a composite + belonging to no real row — `CLAUDE.md` "Grain Collapse". Wrong only when the + duplicate rows are DIFFERENT ENTITIES; `skipna=False` was drafted and + abandoned repo-wide. +- **EHCVM cluster identity**: `v: grappe`, `i: [grappe, menage]` — `CLAUDE.md` + "EHCVM countries". +- **Only 2016-17's cross-sectional half is `cs-17-` prefixed in the wave + scripts; every other half emits the raw wave hhid and relies on `id_walk` / + `panel_ids` chaining** — `Malawi/_/CONTENTS.org`, "plot_features (GH #167)". + Read *before* concluding anything about 2019-20's `i_prefix=''`; it is the + documented decision, not a defect (see §4, trap 3). +- **IHS4's GPS displacement is "nearly but not exactly EA-constant"** — 7 of + 779 EAs carry more than one `lat_modified` — `Malawi/_/CONTENTS.org`, + "GH #323: 2016-17 had no GPS". So the EA-constant-coordinate invariant this + PR asserts holds for 2010-11 and 2019-20 and is *documented not to* for + 2016-17; the test is parametrized accordingly and says why. + +## §4 Invariants & assumptions + +- **A merge key duplicated in BOTH sub-frames is a cartesian by construction** — + `_cartesian_keys` docstring; sound *and* complete. +- **The warm cache hides all of this.** L2-country is written post-collapse. + Every number here was measured cold, in an isolated `LSMS_DATA_DIR` with only + `dvc-cache` symlinked, with `LSMS_NO_CACHE=1`, against #627's core on + `PYTHONPATH` (asserted in-process, not assumed). +- **`Wave.cluster_features()` cannot see a cartesian for Malawi** — site 2 + projects to `(t, v)` first, giving 768 / 819 rows with or without the bug. + Tests must call `grab_data('cluster_features')`. (Guinea-Bissau declares no + `i`, so site 2 does not fire and its wave frame is the merged frame.) +- **An in-process `.first()` patch cannot see a `materialize: make` build** — + hence `runpy` (PR #646's method). All three #637 tables are `materialize: + make`. +- **"Exact duplicates" is not reassurance and invariance can be missingness** — + PR #646 / the Tanzania key. Disposed of here by there being no duplicate + groups at all on a non-null key, which is a stronger statement than "the + duplicates agreed". +- **A BROKEN KEY CAN PRODUCE ZERO DUPLICATES** (#637 trap 3, the Tanzania + `shocks` inversion). A key from the wrong namespace makes every replicated + row its own distinct "household", so the duplicate-count instrument reads + clean on a thoroughly broken key. **"0 duplicates" is therefore necessary, + not sufficient**; the discriminating check is a **per-wave** overlap of the + table's `i` against the roster's — per wave, because clean waves otherwise + carry broken ones through an aggregate check. Run for all three Malawi + sites: **100% in every wave**, matching literally (`101011000014` on both + sides), with no row-count inflation relative to source households × + strategies. Worth running rather than waving away: Malawi 2019-20 has the + surface shape of a namespace split — `cs_i` (`'cs-19-' + format_id`) in + `data_info.yml` for `sample` / `household_roster` / `cluster_features` vs. + `i_prefix=''` in all three wave scripts. **`CONTENTS.org` already records + that asymmetry as deliberate** (§3 above), and the measurement confirms it: + the prefix survives to the API on neither side (0 of 14,612 ids carry it, in + any table), so both are in the raw `case_id` namespace and they agree. + +## §5 Reuse decision + +| quantity | decision | reason | +|---|---|---| +| Malawi `df_geo` key | **re-key** `v: ea_id` → `i: case_id` (`cs_i` in 2019-20), `merge_on: [i]` | files are exactly 1:1 on `case_id`; #627's first named remedy | +| Malawi: delete the `dfs:` block (Mali's cure) | **rejected** | lat/lon exist only in the geovariables file; the cover page has none | +| Guinea-Bissau: re-key on `i` | **rejected** | the GPS file is genuinely grappe-grain and carries nothing finer; both copies of every duplicate agree on `vague` | +| Guinea-Bissau: single-file cure | **rejected** | cover page has no GPS columns at all (checked) | +| Guinea-Bissau: `df_geo` hook + `drop_duplicates()` | **taken** | #627's *second* named remedy — reduce the sub-frame to merge-key grain BEFORE the merge. Lossless: the 5 extra rows are byte-identical incl. GPS timestamp | +| Guinea-Bissau: `groupby().first()` / `keep='first'` | **rejected** | would silently choose between two disagreeing fixes if one ever ships; `drop_duplicates()` lets the guard fire instead | +| Guinea-Bissau: convert to a `materialize: make` script | **rejected** | far heavier than 59 rows warrant, and moves a YAML-expressible table onto the script path against the repo's stated preference | +| `.first(skipna=False)` at any #637 site | **rejected** | no multi-row groups exist; the repo abandoned that approach | +| `aggregation:` key anywhere | **rejected** | dead config, D1 | + +## §6 Open questions for the human + +- **The Guinea-Bissau hook is dispatched BY NAME off the sub-frame key.** This + is real, current behaviour on both `development` and #627's branch, but it is + undocumented and there was **no prior art for it** (`rg '^def df_geo'` across + all 40 countries: zero hits). It is heavily documented in three places and + pinned by a test, but a reviewer may prefer a different mechanism. If so, the + alternatives are ranked in §5. +- **2019-20 at HOUSEHOLD grain**: 8 IHS5 geo rows have a NULL coordinate while + their 15 EA siblings carry the EA's fix. Under the `v`-merge those 8 + households borrowed a sibling's value; under the `i`-merge they get their own + NULL. The returned `(t, v)` table is unchanged (`.first()` skips NA), and the + household-grain intermediate is now honest — but it *is* a behavioural + difference, and it is the only one. +- **`food_coping` / `months_food_inadequate` build only the cross-sectional + half in 2016-17 and 2019-20** (12,447 and 11,434 households; the IHPS panel + halves are absent). Observed while auditing #637; a coverage gap, not a key + defect, and out of scope here. + +--- +### Phase 3 — verification + +- `Malawi/{2010-11,2019-20}/_/data_info.yml` — **OK (§3, §5)**: `merge_on: [i]`, + no `aggregation:`, cured at the merge. Phantom 183,812 → 0 and 171,230 → 0. +- `Guinea-Bissau/2018-19/_/{data_info.yml,mapping.py}` — **OK (§3, §5)**: + reduced before the merge, de-duplication not aggregation. Phantom 59 → 0. +- Value preservation — **OK (§4)**: country-level `cluster_features` + bit-for-bit identical for both countries (`DataFrame.equals` True, index + equal, dtypes equal; Malawi 3,235 x 5, Guinea-Bissau 450 x 4). +- `tests/test_gh323_malawi_gb_cartesian.py` — **OK (§4)**: asserts on + `grab_data`, not `Wave.cluster_features()`. **Negative control run** (pre-fix + configs restored, fresh isolated data root): `6 failed, 10 passed` — the three + cartesian tests report 5410 / 196083 / 185842 and the three structural tests + fail, while the 10 source-invariant and cluster-count tests pass *with the bug + fully present*, which is exactly the blindness the module docstring warns + about. After: `16 passed`. +- `Malawi/_/malawi.py` — **OK (§3)**: no `skipna=False`, no reducer added; one + `dropna(subset=['plot_id'])` that makes an existing silent deletion explicit + and is a provable no-op (`groupby(dropna=True)` already removed those rows). diff --git a/lsms_library/countries/Guinea-Bissau/2018-19/_/data_info.yml b/lsms_library/countries/Guinea-Bissau/2018-19/_/data_info.yml index 118d8cfb0..101a84ef6 100644 --- a/lsms_library/countries/Guinea-Bissau/2018-19/_/data_info.yml +++ b/lsms_library/countries/Guinea-Bissau/2018-19/_/data_info.yml @@ -56,6 +56,31 @@ cluster_features: urbain: Urban URBAIN: Urban df_geo: + # GH #323 site 4 / GH #627 -- CARTESIAN, CURED BEFORE THE MERGE. + # + # `grappe_gps_gnb2018.dta` ships 450 rows for 445 grappes: five + # grappes appear TWICE as VERBATIM duplicate records (identical lat, + # lon, accuracy, altitude and GPS timestamp). df_main is the + # household cover page at ~12 rows per grappe, so merging on `v` when + # `v` repeats on BOTH sides paired 11-12 household rows with 2 GPS + # rows for each of those five: 5,351 -> 5,410 rows, 59 phantom. + # + # Guinea-Bissau is NOT the Malawi/Mali shape. There is no household + # id to re-key on (the GPS file is genuinely at grappe grain and + # carries only `grappe` + `vague`), and no single-file cure (the + # cover page has no GPS columns at all -- checked). So this cell + # takes the OTHER remedy #627's guard names: reduce the sub-frame to + # the merge-key grain BEFORE the merge. The `df_geo` hook in + # ../_/mapping.py does it with drop_duplicates() over every column, + # which removes only rows identical to one already present -- so two + # GPS fixes that actually DISAGREE would both survive and re-trip the + # guard rather than being silently reduced to one. READ THAT + # DOCSTRING: the hook is dispatched by matching this sub-frame's key + # name, which is not obvious from either file alone. + # + # Not fixed here: 5 of the 450 grappes on the cover page have no GPS + # record at all, so they carry NaN coordinates. Absent data, not + # manufactured data. file: grappe_gps_gnb2018.dta idxvars: v: grappe diff --git a/lsms_library/countries/Guinea-Bissau/2018-19/_/mapping.py b/lsms_library/countries/Guinea-Bissau/2018-19/_/mapping.py index 0d4e900ad..74d890029 100644 --- a/lsms_library/countries/Guinea-Bissau/2018-19/_/mapping.py +++ b/lsms_library/countries/Guinea-Bissau/2018-19/_/mapping.py @@ -5,6 +5,61 @@ from lsms_library.transformations import food_acquired_to_canonical as _food_acquired_canonical +def df_geo(df): + """``cluster_features.df_geo`` sub-frame hook — GH #323 site 4 / GH #627. + + **This is a SUB-FRAME hook, not a table hook.** ``Wave.grab_data`` + resolves a ``dfs:`` sub-frame's ``df_edit`` by calling + ``column_mapping(, ...)``, so a module-level function + named exactly after the sub-frame key (``df_geo``) runs on that + sub-frame **after extraction and before the merge**. That "before the + merge" is the whole point; see below. + + ``grappe_gps_gnb2018.dta`` ships **450 rows for 445 grappes**: five + grappes (155325, 155426, 199719, 199816, 1771231) appear TWICE, as + VERBATIM duplicate records — same latitude, same longitude, same + accuracy, same altitude, same GPS timestamp. The companion community + cover ``s00a_co_gnb2018.dta`` carries the identical 450/445 split, so + the duplication happened once, on the community side of the export. + + That alone made the ``cluster_features`` merge a CARTESIAN PRODUCT. + The other sub-frame (``s00_me_gnb2018.dta``, the household cover page) + is HOUSEHOLD grain — ~12 rows per grappe — so for those five grappes + the merge on ``v`` paired 11-12 household rows with 2 GPS rows and + emitted 22-24 where an honest join emits 11-12: **5,351 -> 5,410 rows, + 59 of them phantom** (PR #627's 40-country census). + + The cure is the second remedy the #627 guard itself names — *"reduce a + sub-frame to the merge-key grain BEFORE it is merged"*. It is + deliberately NOT the first one (*"merge on a key unique in at least one + sub-frame"*): Guinea-Bissau has no such key. The GPS file is at grappe + grain and carries nothing finer (only ``grappe`` and ``vague``, and both + copies of every duplicate agree on ``vague``), while the cover page is + at household grain, so ``v`` is non-unique on *both* sides and no + declarable key separates the two copies. + + ``drop_duplicates()`` over EVERY column — key and payload alike — is + used on purpose, and it is de-duplication, not aggregation (#323 D1 + forbids the latter). It removes a row only when an identical row + already exists, so it cannot choose between disagreeing records: if a + future export ever ships two GPS fixes for one grappe that actually + DIFFER, both survive, the merge is cartesian again, and #627's guard + fires. A ``groupby().first()`` or ``keep='first'`` would silently pick + one and hide exactly that. + + Unrelated to the cardinality and NOT fixed here: the household cover + page names 450 grappes but the GPS file only 445, so five grappes carry + NaN coordinates. Missing data, not manufactured data — and note the + two counts coincide only by arithmetic accident: the five duplicated + records are byte-identical, so they are not five mislabelled fixes that + could be reassigned to the five grappes that lack one. + """ + names = list(df.index.names) + flat = df.reset_index() + deduped = flat.drop_duplicates() + return deduped.set_index(names) + + # Lossy-substitution prefixes the source data carries for two specific # Portuguese words. Upstream of pyreadstat, the export pipeline # replaced the second byte of certain UTF-8 codepoints with literal diff --git a/lsms_library/countries/Guinea-Bissau/_/CONTENTS.org b/lsms_library/countries/Guinea-Bissau/_/CONTENTS.org index be72b83d5..49a653dd7 100644 --- a/lsms_library/countries/Guinea-Bissau/_/CONTENTS.org +++ b/lsms_library/countries/Guinea-Bissau/_/CONTENTS.org @@ -13,6 +13,57 @@ each grappe is visited in exactly one =vague=, so household identifier is =(grappe, menage)= without a vague level. See CLAUDE.md "EHCVM countries" for the cross-country pattern. +** =grappe_gps_gnb2018.dta= ships five duplicated records (GH #323 site 4 / #627) + +The GPS file has *450 rows for 445 grappes*. Five grappes -- 155325, +155426, 199719, 199816 and 1771231 -- appear *twice*, as verbatim +duplicate rows: same latitude, same longitude, same accuracy, same +altitude, same GPS timestamp. The companion community cover +=s00a_co_gnb2018.dta= carries the identical 450/445 split, so the +duplication happened once, on the community side of the export. + +That alone made =cluster_features= a *cartesian product*. The other +=dfs:= sub-frame is the household cover page =s00_me_gnb2018.dta= at +~12 rows per grappe, so merging on =v= when =v= repeats on *both* +sides paired 11-12 household rows against 2 GPS rows for each of those +five: *5,351 -> 5,410 rows, 59 phantom* (PR #627's census). Small, but +the same disease as Malawi's 183,812. + +*Guinea-Bissau is not the Malawi shape and does not take the Malawi +cure.* There is no household id to re-key on -- the GPS file is +genuinely at grappe grain and carries nothing finer than =grappe= + +=vague=, and both copies of every duplicate agree on =vague=. Nor is +there Mali 2021-22's single-file cure: the cover page has *no GPS +columns at all* (checked). So this cell takes the other remedy #627's +guard names -- *reduce the sub-frame to the merge-key grain before it +is merged* -- via a =df_geo(df)= hook in =2018-19/_/mapping.py=. + +Two things about that hook are worth knowing before touching it: + +- It is dispatched *by name off the sub-frame key*. =Wave.grab_data= + resolves a =dfs:= sub-frame's =df_edit= with + =column_mapping(, ...)=, so the function must be + called exactly =df_geo= and the sub-frame must stay keyed =df_geo=. + Renaming either silently drops the de-dup. A test pins both. +- It uses =drop_duplicates()= over *every* column, key and payload + alike. That is de-duplication, not aggregation (#323 D1 forbids the + latter): a row is removed only when an identical row already exists, + so if a future export ever ships two GPS fixes for one grappe that + actually *differ*, both survive, the merge is cartesian again, and + #627's guard fires. =groupby().first()= or =keep='first'= would + silently pick one and hide exactly that. + +After: 5,351 wave rows, *0 phantom*, and the returned 450-row +=cluster_features= frame is bit-for-bit identical (=DataFrame.equals= +True, index equal, dtypes equal). + +Separately, and *not* fixed: the cover page names 450 grappes but the +GPS file only 445, so five grappes carry NaN coordinates. Absent data, +not manufactured data. The two counts coincide only by arithmetic +accident -- the five duplicated records are byte-identical, so they are +*not* five mislabelled fixes that could be reassigned to the five +grappes that lack one. + * Household Presence / MonthsSpent The EHCVM questionnaire (used in the 2018-19 wave) does NOT ask diff --git a/lsms_library/countries/Malawi/2010-11/_/data_info.yml b/lsms_library/countries/Malawi/2010-11/_/data_info.yml index 578910820..9eb7e5d71 100644 --- a/lsms_library/countries/Malawi/2010-11/_/data_info.yml +++ b/lsms_library/countries/Malawi/2010-11/_/data_info.yml @@ -129,31 +129,40 @@ cluster_features: # lookups for this wave. District: hh_a01 df_geo: - # GH #323 SITE 4 -- KNOWN CARTESIAN, DELIBERATELY NOT FIXED HERE. - # `householdgeovariables.dta` is HOUSEHOLD grain (12,271 rows, one per - # case_id, across 768 EAs), and this block keys it on `ea_id` and merges - # on the CLUSTER key `v`. `pd.merge` therefore pairs every household in - # df_main with every geo row sharing that `v`: 12,271 x ~16 = 196,083 - # rows, 183,812 of them phantom, for a table that collapses to 768 - # clusters. It costs 16x the time and memory and manufactures the - # duplicates Site 2 then destroys. + # GH #323 SITE 4 / GH #627 -- CARTESIAN, CURED AT THE MERGE. # - # It does NOT corrupt any value: `lat_modified` is constant within every - # ea_id, and a cartesian repeats values rather than inventing them, so - # the contested-cell count is provably unchanged by it (measured: 0 - # before and after collapsing the merge to household grain). The - # one-line fix is `idxvars: {i: case_id}` + `merge_on: [i]` -- the two - # files are exactly 1:1 on case_id, no rows on either side alone. Left - # in place so that PR #627's cardinality guard (core, Site 4) still has - # this cell as evidence; #627 owns the decision. + # This block USED to key `householdgeovariables.dta` on `v: ea_id` and + # merge on the CLUSTER key `v`. Both sub-frames are HOUSEHOLD grain + # (12,271 rows each, one per case_id, across 768 EAs), so `pd.merge` + # paired every household of df_main with every geo row sharing that + # `v`: 196,083 rows, 183,812 of them PHANTOM -- rows that exist in no + # survey. `_normalize_dataframe_index` then mopped the wreck up with + # groupby().first() down to 768 clusters, so the table read clean at + # every point anyone ever looked. + # + # The geo file carries `case_id`, so the honest key was there all + # along. Verified cold against the two source files: + # * exactly 1:1 on case_id -- 12,271 = 12,271, zero rows on either + # side alone, no duplicate case_id in either file; + # * lat_modified / lon_modified are CONSTANT within all 768 ea_id + # (0 EAs with more than one distinct coordinate) -- the published + # GPS is the EA's displaced fix stamped on each household; + # * df_main.ea_id == df_geo.ea_id for all 12,271 households. + # So keying on `i` hands each household its own coordinate, which is + # the coordinate the `v`-merge was already giving it: 196,083 -> 12,271 + # wave rows, 183,812 -> 0 phantom, and every returned value unchanged. + # + # `v` is deliberately NOT declared here. df_main owns it; declaring it + # on both sides of a merge keyed on `i` would only produce a v_x/v_y + # collision. file: Full_Sample/Geovariables/HH_level/householdgeovariables.dta idxvars: - v: ea_id + i: case_id myvars: Latitude: lat_modified Longitude: lon_modified merge_on: - - v + - i final_index: - t - v diff --git a/lsms_library/countries/Malawi/2019-20/_/data_info.yml b/lsms_library/countries/Malawi/2019-20/_/data_info.yml index 40d8af22f..3f1e48cba 100644 --- a/lsms_library/countries/Malawi/2019-20/_/data_info.yml +++ b/lsms_library/countries/Malawi/2019-20/_/data_info.yml @@ -73,32 +73,49 @@ cluster_features: # sign-flipping the indicator between waves. GH #602. Rural: reside df_geo: - # GH #323 SITE 4 -- KNOWN CARTESIAN, DELIBERATELY NOT FIXED HERE. - # `householdgeovariables_ihs5.dta` is HOUSEHOLD grain (11,434 rows, one - # per case_id, across 717 EAs), and this block keys it on `ea_id` and - # merges on the CLUSTER key `v`: 14,612 households x 11,434 geo rows - # paired within `v` = 185,842 rows, 171,230 of them phantom, for a table - # that collapses to 819 clusters. + # GH #323 SITE 4 / GH #627 -- CARTESIAN, CURED AT THE MERGE. # - # It does NOT corrupt any value -- a cartesian repeats values rather - # than inventing them, and the contested-cell count is provably - # unchanged by it (measured: 250 before and after collapsing the merge - # to household grain). The one-line fix is `idxvars: {i: [case_id, - # {function: cs_i}]}` + `merge_on: [i]`; the two files are exactly 1:1 - # on case_id. Left in place so PR #627's cardinality guard (core, - # Site 4) still has this cell as evidence; #627 owns the decision. + # This block USED to key `householdgeovariables_ihs5.dta` on + # `v: ea_id` and merge on the CLUSTER key `v`. Both sub-frames are + # HOUSEHOLD grain -- df_main is 14,612 households (11,434 IHS5 + # cross-section + 3,178 IHPS panel), df_geo 11,434, one row per + # case_id across 717 EAs -- so `pd.merge` paired every household with + # every geo row sharing its `v`: 185,842 rows, 171,230 of them PHANTOM. + # `_normalize_dataframe_index` then collapsed that to 819 clusters with + # groupby().first(), so nothing downstream ever saw it. # - # Unrelated to the cardinality: the 102 IHPS Panel EAs share NO ea_id - # with the geo file, so those clusters have NaN coordinates either way - # -- IHS5 publishes no geovariables for the panel half. + # The geo file carries `case_id`, so the honest key was there all + # along. Verified cold against the two source files: + # * the CROSS-SECTIONAL half is exactly 1:1 on case_id -- 11,434 = + # 11,434, zero rows on either side alone, no duplicates; + # * ea_lat_mod / ea_lon_mod are CONSTANT within all 717 ea_id (0 EAs + # with more than one distinct coordinate) -- the published GPS is + # the EA's displaced fix stamped on each household; + # * df_main.ea_id == df_geo.ea_id for all 11,434 shared households. + # So keying on `i` hands each household its own coordinate, which is + # the coordinate the `v`-merge was already giving it: 185,842 -> 14,612 + # wave rows, 171,230 -> 0 phantom, every returned value unchanged. + # + # `i` MUST be built exactly as df_main builds it -- case_id through + # `cs_i` -- or the join silently matches nothing (the failure mode + # 2016-17 was in until GH #323: 0 of 880 clusters carried GPS). + # `v` is deliberately NOT declared here; df_main owns it. + # + # Unchanged by this fix: the 102 IHPS Panel EAs share NO ea_id and no + # id with the geo file (measured: 0 overlap on either), so the panel + # half's clusters carry NaN coordinates before and after -- IHS5 + # publishes no geovariables for the panel. Under `outer` they arrive + # from df_main alone, exactly as they did under the `v` merge. file: Cross_Sectional/householdgeovariables_ihs5.dta idxvars: - v: ea_id + i: + - case_id + - function: cs_i myvars: Latitude: ea_lat_mod Longitude: ea_lon_mod merge_on: - - v + - i final_index: - t - v diff --git a/lsms_library/countries/Malawi/_/CONTENTS.org b/lsms_library/countries/Malawi/_/CONTENTS.org index 82a14f71a..c2500d4c3 100644 --- a/lsms_library/countries/Malawi/_/CONTENTS.org +++ b/lsms_library/countries/Malawi/_/CONTENTS.org @@ -2,6 +2,119 @@ Brief table of contents and todo list. Malawi has 5 rounds of data, corresponding to IHS2-IHS5 plus IHPS. +* The three =groupby().first()= sites in =malawi.py= (GH #637) + +Reviewed for *key soundness* -- the only thing that makes a +=.first()= wrong. =.first()= defaults to =skipna=True=, so a group of +several rows collapses to a *composite* assembled from the first +non-null value of each column independently, i.e. a row that appears +nowhere in the survey. That is *usually correct*: NULL is absence, not +contradiction. It is wrong only when the duplicate rows describe +*different real entities*, which is a broken identifier and is fixed at +the identifier (#323 D1), never by =skipna=False=. + +*Method.* =DataFrameGroupBy.first= was patched in-process and the wave +scripts driven with =runpy=. All three tables are =materialize: make=, +so an in-process patch cannot see them if they are built through the +subprocess -- an earlier probe of a site like this reported a +meaningless "0" for exactly that reason. Run cold, in an isolated +=LSMS_DATA_DIR=. + +| site (=malawi.py=) | groups >1 row | verdict | +|-------------------------------+---------------+-------------------------------| +| =plot_features_for_wave= | 0 | key sound; =.first()= no-op | +| =food_coping_for_wave= | 0 | key sound; =.first()= no-op | +| =months_food_inadequate_for_wave= | 0 | key sound; =.first()= no-op | + +*No composite is ever built at any of the three*, because no group ever +has more than one row to build one from. For the two Module-H sites +the rows in equal the groups out *exactly* -- 61,355 / 19,994 / 62,232 +/ 57,157 for =food_coping= and 12,271 / 4,000 / 12,447 / 11,434 for +=months_food_inadequate=, the latter being precisely each wave's Module +H household count. + +** The three traps, and which check answers each + +*"They're only exact duplicates" is not reassurance* and *a key can +look invariant merely by being null*. Both are disposed of by the same +exact equality above: a household id shared by two rows, or two +distinct ids merged by =format_id=, would each surface as a duplicate +group, and there are none; and no key column is null in either +Module-H table. Confirmed at source: =case_id= / =y2_hhid= are unique +in Module H and =format_id= collapses no two of them. + +*A broken key can produce ZERO duplicates* -- the Tanzania inversion, +which turns the duplicate-count instrument inside out. +=Tanzania/2008-15='s =shocks= built =i= from the panel-LINE index +instead of the household id; the namespaces shared no values, so every +replicated line became its own distinct "household" and +=groupby().first()= reported 0 duplicate groups, indistinguishable +from a sound key. So *"0 duplicates" is not sufficient*. The check +that catches it is a *per-wave* namespace overlap against the roster -- +per wave, never in aggregate, because clean waves otherwise carry +broken ones. Run for all three tables: + +| table | 2010-11 | 2013-14 | 2016-17 | 2019-20 | +|------------------------+---------+---------+---------+---------| +| plot_features | 100% | 100% | 100% | 100% | +| food_coping | 100% | 100% | 100% | 100% | +| months_food_inadequate | 100% | 100% | 100% | 100% | + +And not vacuously: the id strings match literally (=101011000014= on +both sides), and the row counts show no namespace inflation -- rows +never exceed source households x strategies. + +This was worth running rather than waving away, because *2019-20 has +the surface shape of a namespace split*: its =data_info.yml= runs +=case_id= through =cs_i= (='cs-19-' + format_id=) for =sample= / +=household_roster= / =cluster_features=, while all three wave scripts +pass =i_prefix=''= / bare =format_id=. + +*That asymmetry is a deliberate, already-recorded decision, not a +defect* -- see "plot_features (GH #167)" below: "2016-17 cross-sectional +id is =cs-17-=-prefixed in =sample()= ...; the wave script applies the +same prefix so XS plots are not ~100% orphaned. *All other halves emit +the raw wave hhid* and rely on framework id_walk / =panel_ids= +chaining." 2019-20 is one of those other halves. The measurement +confirms the documented expectation rather than contradicting it: the +=cs-19-= prefix survives to the API on *neither* side (0 of 14,612 ids +carry it, in any table), so both sides are in the raw =case_id= +namespace and =i= joins exactly. 2016-17 reaches the same place by the +other route -- prefixed on both sides. A test pins the overlap per +wave so neither route can drift. + +=plot_features= showed 4 apparent duplicate groups (3 in 2016-17 +Cross_Sectional, 1 in its Panel half). Every one of them consists +entirely of rows whose =plot_id= is *NULL* -- there are *zero* +duplicate groups on a real plot id in any wave, so =(hhid, plotkey)= +identifies the plot and no two plots collide on it. + +** What the audit did find: 468 rows deleted on a null key + +=groupby()= defaults to =dropna=True=, so a =plot_features= row whose +=plot_id= is NULL was being *deleted without a word* -- GH #323 §3b's +delete-and-report, in a country script rather than in core: + +| wave | null plot_id rows | +|---------+-------------------| +| 2010-11 | 275 | +| 2013-14 | 158 | +| 2016-17 | 27 + 6 | +| 2019-20 | 0 + 2 | +| total | 468 | + +They are empty Module C roster stubs, not plots: *0 of all 468* carries +a non-null value in any of =ag_c04a= / =ag_c04b= / =ag_c04c= -- no +farmer-estimated area, no unit, no GPS area. A row with no plot id and +no measurement cannot go into a =(t, i, plot_id)= table, so dropping it +is right; it just should not have been silent. The drop is now an +explicit =dropna(subset=['plot_id'])= immediately before the groupby, +which leaves the returned frame bit-for-bit what it was. + +*No =.first(skipna=False)= anywhere.* It would change nothing here +(there are no multi-row groups) and the repo has already rejected that +approach. + * plot_features (GH #167) Lasting plot-level characteristics. Schema: =(t, i, plot_id)= index; @@ -454,7 +567,10 @@ Malawi has five waves from two survey programs: - IHS3 (2010-11): cross-sectional, 12,271 households - IHPS (2013-14): panel only (tracked from IHS3), 4,000 households - IHS4 (2016-17): cross-sectional (12,447) + panel (2,508) = 14,955 households -- IHS5 (2019-20): cross-sectional (11,434) + panel (3,178) = 14,552 households +- IHS5 (2019-20): cross-sectional (11,434) + panel (3,178) = 14,612 households + (this line read "= 14,552" until 2026-07; the two addends were right and + the total was a typo. 14,612 is what =sample()=, =household_roster()= and + =cluster_features='s =df_main= all return for the wave.) ** Strata District serves as the stratification variable across all waves (~30-32 @@ -580,23 +696,50 @@ Cost of that fix, stated plainly: 7 of the 779 EAs have households whose IHS4 exactly EA-constant), so =Latitude= / =Longitude= now show 7 contested cells where before there were none -- because before there was no coordinate at all. -*** Known, deliberately unfixed: two cartesian =df_geo= merges (Site 4, PR #627) -2010-11 and 2019-20 key a *household-grain* geovariables file on =ea_id= and -merge it on the *cluster* key =v=, so =pd.merge= pairs every household with -every geo row sharing that =v=: - -| wave | households | geo rows | merged rows | phantom | -|---------+------------+----------+-------------+---------| -| 2010-11 | 12,271 | 12,271 | 196,083 | 183,812 | -| 2019-20 | 14,612 | 11,434 | 185,842 | 171,230 | - -A cartesian *repeats* values, it does not invent them, so it corrupts nothing -and changes no contested-cell count -- verified by measuring both waves with -the merge collapsed to household grain (=merge_on: [i]=, the two files are 1:1 -on =case_id=): 0 and 250 contested cells respectively, identical either way. -It does cost roughly 16x the build time and memory. The one-line config fix is -noted in each wave's =data_info.yml= but *not applied here*, so that PR #627's -cardinality guard keeps these cells as evidence. #627 owns the decision. +*** FIXED: two cartesian =df_geo= merges (Site 4, PR #627) +2010-11 and 2019-20 *used to* key a *household-grain* geovariables file on +=ea_id= and merge it on the *cluster* key =v=, so =pd.merge= paired every +household with every geo row sharing that =v=: + +| wave | households | geo rows | merged rows | phantom | after | +|---------+------------+----------+-------------+---------+--------| +| 2010-11 | 12,271 | 12,271 | 196,083 | 183,812 | 12,271 | +| 2019-20 | 14,612 | 11,434 | 185,842 | 171,230 | 14,612 | + +Between them, 355,042 rows that exist in no survey -- 7.2% of the 4,907,774 +phantom rows PR #627's 40-country census found. =_normalize_dataframe_index= +then mopped the wreck up with =groupby().first()= down to 768 / 819 clusters, +so the table read clean at every point anyone ever looked. + +Both geo files carry =case_id=, so the honest key was there all along. Both +are now =idxvars: {i: case_id}= + =merge_on: [i]= (2019-20's runs =case_id= +through =cs_i=, exactly as its =df_main= does -- get that wrong and the join +matches nothing, which is the failure 2016-17 was in). Measured cold, in an +isolated data root, with #627's cardinality guard on: *phantom rows 183,812 -> +0 and 171,230 -> 0*. + +*The values do not move.* The published lat/lon is the EA's *displaced fix +stamped on every household of the EA*, not a household fix -- verified: 0 of +768 EAs (2010-11) and 0 of 717 (2019-20) carry more than one distinct +coordinate, the two files are exactly 1:1 on =case_id= with no rows on either +side alone, and =df_main.ea_id == df_geo.ea_id= for every shared household. So +handing each household its own row returns the coordinate the =v=-merge was +already returning it. The full country-level =cluster_features= frame (3,235 +rows x 5 columns, all five waves) is *bit-for-bit identical* before and after: +=DataFrame.equals= True, index equal, dtypes equal. + +One honest wrinkle, at *household* grain only: 8 rows of the IHS5 geo file have +a NULL coordinate while their 15 EA siblings carry the EA's fix. Under the +=v=-merge those 8 households borrowed a sibling's value; under the =i=-merge +they get their own NULL. =cluster_features= is returned at =(t, v)= grain and +=.first()= skips NA, so the cluster still reports the EA's coordinate and the +returned table is unchanged -- but the household-grain intermediate now tells +the truth about which rows the survey actually geolocated. + +Unchanged and *not* a defect: the 102 IHPS panel EAs of 2019-20 share no +=ea_id= and no household id with the geo file (0 overlap on either), because +IHS5 publishes no geovariables for the panel half. Those clusters carry NaN +coordinates before and after. ** Weights | Wave | Cross-section weight | Panel weight | Source file | diff --git a/lsms_library/countries/Malawi/_/malawi.py b/lsms_library/countries/Malawi/_/malawi.py index 35748f7b2..2c2231357 100755 --- a/lsms_library/countries/Malawi/_/malawi.py +++ b/lsms_library/countries/Malawi/_/malawi.py @@ -833,8 +833,42 @@ def plot_features_for_wave(t, df_c, df_d, colmap): 'Fallow': fallow.values, 'ErosionProtection': erosion_protection.values, }) - # Collapse any duplicate (hhid, plotkey) area rows defensively - # (Module C should be one row per plot; first-wins keeps it unique). + # GH #637 -- what this groupby actually does, measured rather than assumed. + # + # Audited cold across all six (wave x half) builds by patching + # DataFrameGroupBy.first in-process and driving the wave scripts with + # runpy (an in-process patch cannot see a `materialize: make` subprocess). + # Result: ZERO groups of more than one row on a non-null plot_id, in every + # wave. So `.first()` collapses nothing real and cannot assemble the + # skipna COMPOSITE that #637 is about -- there are no duplicate rows for it + # to compose. Module C is one row per plot, and (hhid, plotkey) identifies + # that plot: no two plots collide on it, and format_id merges no two + # distinct ids (both would show up here as duplicate groups, and neither + # does). Left in place as a guard, not removed: it is free, and the wave + # scripts' `assert df.index.is_unique` depends on the frame being unique. + # + # "0 duplicates" ALONE would not license that verdict -- a key drawn from + # the WRONG NAMESPACE also yields 0, because every row becomes its own + # distinct "household" (the Tanzania shocks inversion, #637 trap 3). So + # the namespace was checked separately and PER WAVE against the roster: + # 100% of `i` present in household_roster for 2010-11 / 2013-14 / 2016-17 / + # 2019-20 alike, matching literally rather than vacuously. Pinned by + # tests/test_gh637_malawi_first_sites.py. + # + # What the audit DID find is a silent deletion, which is now explicit. + # `groupby()` defaults to dropna=True, so a row whose plot_id is NULL was + # being DROPPED here without a word (GH #323 §3b's delete-and-report, in a + # country script rather than in core): + # + # 2010-11 275 2013-14 158 2016-17 27 + 6 2019-20 0 + 2 = 468 + # + # Those rows are empty Module C roster stubs, not plots: 0 of all 468 + # carries a non-null value in ANY of ag_c04a / ag_c04b / ag_c04c, i.e. no + # farmer-estimated area, no unit and no GPS area. A row with no plot id + # and no measurement cannot go into a (t, i, plot_id) table, so dropping it + # is right -- but it should say so. Dropping them here first leaves the + # returned frame bit-for-bit what it was. + out = out.dropna(subset=['plot_id']) out = out.groupby(['t', 'i', 'plot_id'], as_index=False).first() out = out.set_index(['t', 'i', 'plot_id']) return out @@ -1129,7 +1163,18 @@ def food_coping_for_wave(t, df, idcol, i_prefix=''): out['t'] = t out = out.dropna(subset=['Days']) out['Days'] = out['Days'].round().astype('Int64') - # One row per (HH, strategy); first-wins guards against duplicate HH rows. + # One row per (HH, strategy). GH #637: audited cold in all four waves by + # patching DataFrameGroupBy.first in-process and driving the wave scripts + # with runpy -- rows in == groups out EXACTLY (61,355 / 19,994 / 62,232 / + # 57,157), zero groups of more than one row, zero rows with a null key. + # So this reduces nothing and cannot produce the skipna composite #637 is + # about. That equality is also what rules out two of the three traps: + # `idcol` is one row per household in Module H, and `format_id` merges no + # two distinct ids -- either failure would show up right here as a + # duplicate (i, Strategy) group, and none exists. The third trap needs a + # different instrument: a WRONG-NAMESPACE key also yields 0 duplicates + # (#637 trap 3), so `i` was checked PER WAVE against household_roster -- + # 100% overlap in all four waves. Kept as a guard, not removed. out = out.groupby(['t', 'i', 'Strategy'], as_index=False).first() out = out.set_index(['t', 'i', 'Strategy']) return out @@ -1199,6 +1244,15 @@ def months_food_inadequate_for_wave(t, df, idcol, i_prefix=''): 'MonthsInadequate': months.values, 'AnyInadequate': any_inadequate.values, }) + # GH #637: same audit as food_coping_for_wave above. Rows in == groups out + # exactly in all four waves -- 12,271 / 4,000 / 12,447 / 11,434, which is + # precisely each wave's Module H household count -- so `i` is unique here + # and `.first()` is a no-op. A `.first(skipna=False)` would change nothing + # and is deliberately NOT used; the composite this site could in principle + # build never arises, because there are no duplicate rows to build it from. + # Trap 3 checked here too: `i` overlaps household_roster 100% PER WAVE, so + # the absent duplicates are absent because the key is sound, not because it + # is in the wrong namespace. out = out.groupby(['t', 'i'], as_index=False).first() out = out.set_index(['t', 'i']) return out diff --git a/tests/test_gh323_malawi_cluster_key.py b/tests/test_gh323_malawi_cluster_key.py index 99abad8c9..453a33d12 100644 --- a/tests/test_gh323_malawi_cluster_key.py +++ b/tests/test_gh323_malawi_cluster_key.py @@ -199,38 +199,32 @@ def test_every_wave_uses_the_same_eight_digit_ea_keyspace(sample): # 3. The df_geo merges: no cartesian product, and the key actually matches. # --------------------------------------------------------------------------- -@pytest.mark.parametrize("wave", ["2010-11", "2019-20"]) -def test_the_known_cartesian_geo_merges_are_documented_where_they_live(wave): - """2010-11 and 2019-20 merge a HOUSEHOLD-grain geo file on the CLUSTER key. - - That pairs every household with every geo row sharing its `v`: 196,083 rows - from 12,271 (183,812 phantom) and 185,842 from 14,612 (171,230 phantom). - It is Site 4, it is owned centrally by PR #627's cardinality guard, and it - is deliberately NOT fixed here -- but it must not be silent either. A - cartesian repeats values rather than inventing them, so it changes no - contested-cell count; see the paired test below, which is the load-bearing - claim. - """ - cfg = _wave_config(wave)["cluster_features"] - assert cfg.get("merge_on") == ["v"], ( - f"{wave}: if this now merges on `i` the cartesian has been fixed -- " - "update the note in data_info.yml and tell PR #627 it lost an example" - ) - src = (countries_root() / "Malawi" / wave / "_" / "data_info.yml").read_text() - assert "SITE 4" in src and "CARTESIAN" in src, ( - f"{wave}: the known cartesian df_geo merge must stay documented in " - "data_info.yml, next to the declaration that causes it" - ) - - -@pytest.mark.parametrize("wave", ["2016-17"]) +# A test used to sit here pinning 2010-11 and 2019-20 as KNOWN-cartesian and +# deliberately unfixed, so PR #627's census would keep them as evidence. Its +# own failure message named the condition for retiring it -- "if this now merges +# on `i` the cartesian has been fixed ... tell PR #627 it lost an example" -- and +# that has now happened: both waves merge on `i`, 183,812 and 171,230 phantom +# rows -> 0, values bit-for-bit unchanged. What replaced it is the widened +# parametrize below (the two waves join the third in requiring `df_geo` to build +# `i` exactly as `df_main` does) plus the row-count assertions in +# tests/test_gh323_malawi_gb_cartesian.py, which is where the cartesian itself +# is measured. + + +@pytest.mark.parametrize("wave", ["2010-11", "2016-17", "2019-20"]) def test_geo_subframe_builds_i_exactly_as_the_main_frame_does(wave): """Both halves of IHS4/IHS5 live in one table, so `i` for the Cross_Sectional half is run through `cs_i` to keep it apart from the Panel's y{3,4}_hhid. The geo block declared the RAW case_id, so its merge key never matched: 2016-17 ended up with GPS for 0 of its 880 clusters, plus 12,447 orphan geo rows manufactured by the outer join. The two blocks must build - `i` identically or they drift apart again.""" + `i` identically or they drift apart again. + + Widened to all three household-keyed waves when 2010-11 and 2019-20 stopped + merging on the cluster key (GH #627). For those two the risk runs the other + way from 2016-17's: there the mismatch matched NOTHING, here a mismatch + would silently reintroduce the cartesian. 2010-11 declares a bare + `case_id` on both sides; 2019-20 runs it through `cs_i` on both.""" cfg = _wave_config(wave)["cluster_features"] main_i = cfg["df_main"]["idxvars"].get("i") geo_i = cfg["df_geo"]["idxvars"].get("i") diff --git a/tests/test_gh323_malawi_gb_cartesian.py b/tests/test_gh323_malawi_gb_cartesian.py new file mode 100644 index 000000000..8d682efc0 --- /dev/null +++ b/tests/test_gh323_malawi_gb_cartesian.py @@ -0,0 +1,323 @@ +"""GH #323 site 4 / GH #627 — Malawi's and Guinea-Bissau's `dfs:` cartesians. + +PR #627's 40-country census found 8 many-to-many `dfs:` merges fabricating +4,907,774 phantom rows. Mali's 4.3M went in PR #641; these three are the rest +of the ones nobody owned: + + Malawi 2010-11 12,271 x 12,271 -> 196,083 rows (183,812 phantom) + Malawi 2019-20 14,612 x 11,434 -> 185,842 rows (171,230 phantom) + Guinea-Bissau 2018-19 5,351 x 450 -> 5,410 rows ( 59 phantom) + +They are TWO DIFFERENT SHAPES and take two different cures. + +**Malawi — a household-grain frame keyed on the cluster.** Both sub-frames are +one row per household, merged on `v`, so `pd.merge` paired every household of +an EA with every other. The geo file carries `case_id`, so the honest key was +there all along: `idxvars: {i: case_id}` + `merge_on: [i]`. + +**Guinea-Bissau — a duplicated source record.** Not the Malawi shape. The GPS +file is genuinely at grappe grain; it just ships five grappes TWICE, as +verbatim duplicate rows. There is no household id to re-key on and no +single-file cure (the cover page carries no GPS at all), so this one takes the +guard's other named remedy — reduce the sub-frame to the merge-key grain BEFORE +the merge — via the `df_geo` hook in Guinea-Bissau/2018-19/_/mapping.py. + +WHERE THE TESTS HAVE TO LOOK. Not at the country-level API frame. The +`(t, v)` collapse makes it 768 / 819 / 450 rows with or without the bug, and +its values are bit-for-bit identical before and after (verified cold over the +whole 3,235-row Malawi frame and the 450-row Guinea-Bissau frame: +`DataFrame.equals` True, index equal, dtypes equal). A country-level assertion +therefore passes with the bug fully present — so these tests count rows in the +WAVE-level frame, upstream of the collapse, and assert the source invariants +that license the re-key rather than merely asserting them in a comment. +""" +import pandas as pd +import pytest +import yaml + +try: + # `tests/` is a package and the repo ROOT also has a conftest.py, so a bare + # `from conftest import ...` resolves to the wrong one. + from tests.conftest import requires_s3 +except ImportError: # pragma: no cover + from conftest import requires_s3 +from lsms_library.country import Country +from lsms_library.local_tools import get_dataframe, format_id +from lsms_library.paths import countries_root + +pytestmark = requires_s3 + + +# Wave-level `cluster_features` row counts: the PRE-collapse frames, where a +# cartesian is visible and nowhere else. Each equals the number of households +# the wave's primary sub-frame actually has. +EXPECTED_WAVE_ROWS = { + ('Malawi', '2010-11'): 12271, # was 196,083 + ('Malawi', '2019-20'): 14612, # was 185,842 (11,434 IHS5 + 3,178 IHPS) + ('Guinea-Bissau', '2018-19'): 5351, # was 5,410 +} + +# Cluster counts the collapsed table returns — unchanged by these fixes, and +# pinned so a "fix" that silently drops clusters cannot pass. +EXPECTED_CLUSTERS = { + ('Malawi', '2010-11'): 768, + ('Malawi', '2019-20'): 819, + ('Guinea-Bissau', '2018-19'): 450, +} + + +@pytest.fixture(scope='module') +def countries(): + out = {} + for name in ('Malawi', 'Guinea-Bissau'): + try: + out[name] = Country(name) + except Exception as exc: # pragma: no cover + pytest.skip(f'{name} unavailable: {exc}') + return out + + +def _wave_frame(countries, country, wave): + """The wave frame AS THE MERGE LEFT IT. + + ``grab_data``, not ``Wave.cluster_features``: the latter runs the site-2 + projection onto ``(t, v)`` first, which returns 768 / 819 rows for Malawi + with or without the cartesian and would make this whole module vacuous. + """ + c = countries[country] + if wave not in c.waves: # pragma: no cover + pytest.skip(f'{wave} is not a {country} wave') + try: + return c[wave].grab_data('cluster_features') + except Exception as exc: # pragma: no cover + pytest.skip(f'{country}/{wave} cluster_features could not be built: ' + f'{type(exc).__name__}: {exc}') + + +def _wave_config(country, wave): + path = countries_root() / country / wave / '_' / 'data_info.yml' + if not path.exists(): # pragma: no cover + pytest.skip(f'{country}/{wave} data_info.yml missing') + with open(path) as f: + return yaml.safe_load(f) + + +def _source(country, wave, rel): + try: + return get_dataframe(str(countries_root() / country / wave / 'Data' / rel)) + except Exception as exc: # pragma: no cover + pytest.skip(f'{country}/{wave} {rel} unavailable: ' + f'{type(exc).__name__}: {exc}') + + +# --------------------------------------------------------------------------- +# 1. The assertion that fails with the bug present. +# --------------------------------------------------------------------------- + +@pytest.mark.parametrize('cell', sorted(EXPECTED_WAVE_ROWS)) +def test_wave_cluster_features_is_not_a_cartesian(countries, cell): + """The wave frame must not be finer than the grain of its own sources.""" + country, wave = cell + df = _wave_frame(countries, country, wave) + expected = EXPECTED_WAVE_ROWS[cell] + assert len(df) == expected, ( + f'{country}/{wave} cluster_features returned {len(df)} wave rows, ' + f'expected {expected}. A count far above {expected} means the `dfs:` ' + f'merge is many-to-many again — both sub-frames repeat the merge key, ' + f'so pandas emits a cartesian product within each key group. Fix the ' + f'MERGE (GH #323 D1); do not reduce afterwards.' + ) + + +@pytest.mark.parametrize('cell', sorted(EXPECTED_CLUSTERS)) +def test_cluster_grain_is_unchanged(countries, cell): + """The re-key is value-preserving: same clusters, same count.""" + country, wave = cell + df = _wave_frame(countries, country, wave) + v = df.reset_index()['v'].dropna().astype(str) + assert v.nunique() == EXPECTED_CLUSTERS[cell], ( + f'{country}/{wave} should cover {EXPECTED_CLUSTERS[cell]} clusters, ' + f'got {v.nunique()}' + ) + + +# --------------------------------------------------------------------------- +# 2. Malawi: the config, and the source invariants that license it. +# --------------------------------------------------------------------------- + +@pytest.mark.parametrize('wave', ['2010-11', '2019-20']) +def test_malawi_geo_subframe_is_keyed_on_the_household(wave): + """`df_geo` must join on `i`, not on the cluster `v`. + + Structural, so it fails on the CONFIG rather than waiting for a build. The + inverse of #639's test, which pinned these two cells as *known cartesian* + while #627 still needed them as evidence; #627's census is now recorded and + the cells are cured. + """ + cfg = _wave_config('Malawi', wave)['cluster_features'] + geo = cfg['df_geo'] + assert 'i' in geo['idxvars'], ( + f'Malawi/{wave} df_geo must declare `i` — keying the household-grain ' + f'geo file on the cluster `v` is what made this merge a cartesian' + ) + assert 'v' not in geo['idxvars'], ( + f'Malawi/{wave} df_geo must NOT also declare `v`: df_main owns it, and ' + f'declaring it on both sides of an `i` merge yields a v_x/v_y collision' + ) + assert cfg['merge_on'] == ['i'], ( + f'Malawi/{wave} cluster_features must merge on [i], got {cfg["merge_on"]}' + ) + + +MALAWI_SOURCES = { + '2010-11': ('Full_Sample/Household/hh_mod_a_filt.dta', + 'Full_Sample/Geovariables/HH_level/householdgeovariables.dta', + 'lat_modified', 'lon_modified', 12271), + '2019-20': ('Cross_Sectional/hh_mod_a_filt.dta', + 'Cross_Sectional/householdgeovariables_ihs5.dta', + 'ea_lat_mod', 'ea_lon_mod', 11434), +} + + +@pytest.mark.parametrize('wave', sorted(MALAWI_SOURCES)) +def test_malawi_main_and_geo_are_exactly_one_to_one_on_case_id(wave): + """What makes `merge_on: [i]` an exact join rather than a lossy one. + + If a future release ships a geo extract that is not 1:1 with the cover + page, the outer merge starts emitting orphan rows on a null `v` and this + says so — instead of the orphans quietly collapsing into one phantom + null-keyed cluster. + """ + main_rel, geo_rel, _, _, n = MALAWI_SOURCES[wave] + main = _source('Malawi', wave, main_rel) + geo = _source('Malawi', wave, geo_rel) + m = main['case_id'].apply(format_id).astype(str) + g = geo['case_id'].apply(format_id).astype(str) + assert len(m) == m.nunique() == len(g) == g.nunique() == n, ( + f'Malawi/{wave}: expected {n} unique case_id on both sides, got ' + f'main {len(m)}/{m.nunique()}, geo {len(g)}/{g.nunique()}' + ) + assert set(m) == set(g), ( + f'Malawi/{wave}: {len(set(m) - set(g))} households have no geo row and ' + f'{len(set(g) - set(m))} geo rows have no household' + ) + + +@pytest.mark.parametrize('wave', sorted(MALAWI_SOURCES)) +def test_malawi_published_gps_is_the_cluster_fix_not_a_household_fix(wave): + """Why re-keying on `i` returns exactly the coordinates `v` was returning. + + The published lat/lon is the EA's DISPLACED fix stamped on every household + of the EA, so `first()`-ing over the EA (what the `v` merge did) and taking + the household's own row (what the `i` merge does) agree — for every EA. + That equality is the value-preservation argument; assert it, don't narrate + it. + + **Do not widen this to 2016-17.** `Malawi/_/CONTENTS.org` ("2016-17 had no + GPS, and the reason was a merge key") records that IHS4's displacement is + "nearly but not exactly EA-constant" — 7 of its 779 EAs carry more than one + `lat_modified`. That wave is exempt by measurement, not by oversight, and + it is not one of the cells this PR re-keys. + """ + _, geo_rel, lat, lon, _ = MALAWI_SOURCES[wave] + geo = _source('Malawi', wave, geo_rel) + g = pd.DataFrame({ + 'v': geo['ea_id'].apply(format_id).astype(str), + 'lat': pd.to_numeric(geo[lat], errors='coerce'), + 'lon': pd.to_numeric(geo[lon], errors='coerce'), + }) + spread = g.groupby('v')[['lat', 'lon']].nunique(dropna=True) + bad = spread[(spread['lat'] > 1) | (spread['lon'] > 1)] + assert len(bad) == 0, ( + f'Malawi/{wave}: {len(bad)} EAs carry more than one distinct ' + f'coordinate, so the household\'s own fix and its EA\'s fix are no ' + f'longer the same value and the re-key is NOT value-preserving:\n' + f'{bad.head()}' + ) + + +def test_malawi_2019_20_panel_half_has_no_geovariables(): + """The NaN coordinates on the IHPS half are absent data, not a broken join. + + Pinned because it is the one thing that could be mistaken for damage done + by the re-key: 102 panel EAs carry no coordinates before or after, because + IHS5 publishes no geovariables for the panel. If a future release DOES + ship them, this fails and someone wires them up. + """ + panel = _source('Malawi', '2019-20', 'Panel/hh_mod_a_filt_19.dta') + geo = _source('Malawi', '2019-20', + 'Cross_Sectional/householdgeovariables_ihs5.dta') + pn_ea = set(panel['ea_id'].apply(format_id).astype(str)) + geo_ea = set(geo['ea_id'].apply(format_id).astype(str)) + assert pn_ea and not (pn_ea & geo_ea), ( + f'{len(pn_ea & geo_ea)} IHPS panel EAs now appear in the IHS5 geo ' + f'file; the panel half\'s coordinates are no longer structurally ' + f'absent and cluster_features should carry them' + ) + + +# --------------------------------------------------------------------------- +# 3. Guinea-Bissau: a duplicated record, not a household-grain frame. +# --------------------------------------------------------------------------- + +def test_guinea_bissau_gps_duplicates_are_verbatim(): + """What licenses `drop_duplicates()` in the `df_geo` hook. + + `grappe_gps_gnb2018.dta` ships 450 rows for 445 grappes. The de-dup is + lossless ONLY because the five extra rows are identical in every column, + GPS timestamp included — there is no choice being made between two + disagreeing fixes. If a future export ships two grappe rows that DIFFER, + both survive `drop_duplicates`, the merge is cartesian again, and #627's + guard fires — which is the behaviour we want, and this test states the + premise it rests on. + """ + gps = _source('Guinea-Bissau', '2018-19', 'grappe_gps_gnb2018.dta') + v = gps['grappe'].apply(format_id).astype(str) + assert len(gps) == 450 and v.nunique() == 445, ( + f'expected 450 rows / 445 grappes, got {len(gps)} / {v.nunique()}' + ) + g = gps.copy() + g['_v'] = v.values + for key, sub in g[g['_v'].isin(set(v[v.duplicated(keep=False)]))].groupby('_v'): + assert len(sub.drop(columns='_v').drop_duplicates()) == 1, ( + f'grappe {key} has {len(sub)} GPS rows that are NOT identical; ' + f'de-duplicating them would be a choice between disagreeing fixes, ' + f'which drop_duplicates() deliberately refuses to make' + ) + + +def test_guinea_bissau_cover_page_has_no_gps_of_its_own(): + """Why Guinea-Bissau cannot take Mali's single-file cure. + + Mali 2021-22's `dfs:` block was deleted outright because its cover page + carried the geography itself. Guinea-Bissau's does not, so the merge is + genuinely needed and only its grain could be fixed. + """ + cov = _source('Guinea-Bissau', '2018-19', 's00_me_gnb2018.dta') + gpsish = [c for c in cov.columns + if any(k in c.lower() for k in ('gps', 'lat', 'lon', 'coord'))] + assert not gpsish, ( + f'the cover page now carries {gpsish}; the `dfs:` block may be ' + f'collapsible to a single-file extraction, as Mali 2021-22 was' + ) + + +def test_guinea_bissau_df_geo_hook_is_wired(): + """The hook is dispatched BY NAME off the sub-frame key — pin that. + + `Wave.grab_data` resolves a sub-frame's `df_edit` via + `column_mapping(, ...)`, so the function must be called + exactly `df_geo` and the sub-frame must be keyed exactly `df_geo`. Neither + file says so on its own; a rename of either silently drops the de-dup and + the cartesian returns. + """ + cfg = _wave_config('Guinea-Bissau', '2018-19')['cluster_features'] + assert 'df_geo' in cfg['dfs'], ( + "the sub-frame must stay keyed `df_geo` — the hook is found by this name" + ) + c = Country('Guinea-Bissau') + hook = c['2018-19'].formatting_functions.get('df_geo') + assert callable(hook), ( + 'Guinea-Bissau/2018-19/_/mapping.py must define `df_geo(df)`; without ' + 'it the 5 duplicate GPS records make the merge cartesian again' + ) diff --git a/tests/test_gh637_malawi_first_sites.py b/tests/test_gh637_malawi_first_sites.py new file mode 100644 index 000000000..643e31e29 --- /dev/null +++ b/tests/test_gh637_malawi_first_sites.py @@ -0,0 +1,314 @@ +"""GH #637 — the three `groupby().first()` sites in `Malawi/_/malawi.py`. + +`.first()` defaults to `skipna=True`, so a group of several rows collapses to a +COMPOSITE assembled from the first non-null value of each column independently — +a row that appears nowhere in the survey. That is usually CORRECT (NULL is +absence, not contradiction). It is wrong only when the duplicate rows describe +DIFFERENT REAL ENTITIES, which is a broken identifier and is fixed at the +identifier, never with `skipna=False`. + +The audit (patch `DataFrameGroupBy.first` in-process, drive the wave scripts +with `runpy` — all three tables are `materialize: make`, so an in-process patch +cannot see them through the subprocess) found **zero groups of more than one row +on a non-null key at any of the three sites, in any wave**. No composite is +ever built, because there is never more than one row to build one from. + +These tests pin the invariants that verdict rests on, so a future source release +or `format_id` change that breaks a key is caught rather than silently +`.first()`-ed away. Note what the audit means for the two known traps: + +* *"they're only exact duplicates"* is not reassurance (PR #646 found broken + Uganda keys where most groups classified 'exact' because only the payload + agreed). Not applicable here — there are no duplicate groups AT ALL, which + is a strictly stronger statement than "the duplicates agreed". +* *invariance by missingness* — a key can look clean by being 100% NULL. Not + applicable either: the household id is non-null in every row of all three + tables, and the one key level that IS ever null (`plot_id`) is measured + below rather than assumed away. +* **a broken key can produce ZERO duplicates** — the Tanzania inversion, which + turns the duplicate-count instrument inside out. `Tanzania/2008-15`'s + `shocks` built `i` from the panel-LINE index instead of the household id; + the two namespaces shared *no* values, so every replicated line became its + own distinct "household" and `groupby().first()` reported 0 duplicate + groups — indistinguishable from a perfectly sound key. **"0 duplicates" is + therefore not sufficient**, and the check that catches it is a PER-WAVE + namespace overlap against the roster (`test_i_lives_in_the_same_namespace_ + as_the_roster` below). Malawi passes it at 100% in every wave of all three + tables — and not vacuously: the id strings match literally. + + Worth running rather than waving away, because Malawi 2019-20 has the + surface shape of a namespace split: its `data_info.yml` runs `case_id` + through `cs_i` (`'cs-19-' + format_id`) for `sample` / `household_roster` / + `cluster_features`, while all three wave scripts here pass `i_prefix=''` / + bare `format_id`. That asymmetry is a DELIBERATE, ALREADY-RECORDED decision + — `Malawi/_/CONTENTS.org`, "plot_features (GH #167)": *"2016-17 + cross-sectional id is `cs-17-`-prefixed in `sample()` ...; the wave script + applies the same prefix so XS plots are not ~100% orphaned. All other halves + emit the raw wave hhid and rely on framework id_walk / `panel_ids` + chaining."* Measured, the `cs-19-` prefix survives to the API on NEITHER + side, so both are in the raw `case_id` namespace and `i` joins exactly. The + test below keeps it that way. +""" +import pandas as pd +import pytest + +try: + # `tests/` is a package and the repo ROOT also has a conftest.py, so a bare + # `from conftest import ...` resolves to the wrong one. + from tests.conftest import requires_s3 +except ImportError: # pragma: no cover + from conftest import requires_s3 +from lsms_library.country import Country +from lsms_library.local_tools import get_dataframe, format_id +from lsms_library.paths import countries_root + +pytestmark = requires_s3 + + +# Module C rows, and how many of them carry NO plot id. Those are empty roster +# stubs, not plots: 0 of all 468 has a non-null ag_c04a / ag_c04b / ag_c04c. +# `groupby(dropna=True)` was deleting them silently; the drop is now explicit. +MODULE_C = { + ('2010-11', 'Full_Sample/Agriculture/ag_mod_c.dta', 'case_id', ('ag_c00',), ''): + (19265, 275), + ('2013-14', 'AG_MOD_C_13.dta', 'y2_hhid', ('ag_c00',), ''): + (6489, 158), + ('2016-17', 'Cross_Sectional/ag_mod_c.dta', 'case_id', ('gardenid', 'plotid'), 'cs-17-'): + (15751, 27), + ('2016-17', 'Panel/ag_mod_c_16.dta', 'y3_hhid', ('gardenid', 'plotid'), ''): + (4042, 6), + ('2019-20', 'Cross_Sectional/ag_mod_c.dta', 'case_id', ('gardenid', 'plotid'), 'cs-17-'): + (17693, 0), + ('2019-20', 'Panel/ag_mod_c_19.dta', 'y4_hhid', ('gardenid', 'plotid'), ''): + (5570, 2), +} + +AREA_COLS = ('ag_c04a', 'ag_c04b', 'ag_c04c') + +# Module H is one row per household — the premise under BOTH Module-H sites. +# (2016-17 and 2019-20 build the cross-sectional half only; the IHPS panel +# halves are absent from both tables. A coverage gap, not a key defect.) +MODULE_H = { + '2010-11': ('Full_Sample/Household/hh_mod_h.dta', 'case_id', 12271), + '2013-14': ('HH_MOD_H_13.dta', 'y2_hhid', 4000), + '2016-17': ('Cross_Sectional/hh_mod_h.dta', 'case_id', 12447), + '2019-20': ('Cross_Sectional/HH_MOD_H.dta', 'case_id', 11434), +} + + +@pytest.fixture(scope='module') +def malawi(): + try: + return Country('Malawi') + except Exception as exc: # pragma: no cover + pytest.skip(f'Malawi unavailable: {exc}') + + +def _source(wave, rel, **kw): + try: + return get_dataframe( + str(countries_root() / 'Malawi' / wave / 'Data' / rel), **kw) + except Exception as exc: # pragma: no cover + pytest.skip(f'Malawi/{wave} {rel} unavailable: ' + f'{type(exc).__name__}: {exc}') + + +def _table(malawi, name): + try: + df = getattr(malawi, name)() + except Exception as exc: # pragma: no cover + pytest.skip(f'Malawi {name} unavailable: {type(exc).__name__}: {exc}') + if df is None or df.empty: # pragma: no cover + pytest.skip(f'Malawi {name} empty') + return df + + +def _plotkey(df, cols): + if len(cols) == 1: + return df[cols[0]].apply(format_id).astype('string') + return (df[cols[0]].apply(format_id).astype('string') + '_' + + df[cols[1]].apply(format_id).astype('string')) + + +# --------------------------------------------------------------------------- +# Site 1 — plot_features_for_wave, groupby(['t','i','plot_id']).first() +# --------------------------------------------------------------------------- + +@pytest.mark.parametrize('spec', sorted(MODULE_C)) +def test_plot_key_identifies_the_plot(spec): + """No two Module C rows share an (i, plot_id). + + This is the whole verdict for this site: with no duplicate group there is + nothing for `.first()` to collapse and no composite for it to assemble. It + also disposes of the collision question — two distinct plots given the same + id, or two distinct household ids merged by `format_id`, would BOTH appear + here as a duplicate group. + """ + wave, rel, idcol, keycols, prefix = spec + df = _source(wave, rel, convert_categoricals=False) + k = pd.DataFrame({ + 'i': prefix + df[idcol].apply(format_id).astype('string'), + 'plot_id': _plotkey(df, keycols), + }) + k = k[k['plot_id'].notna()] + dup = k.groupby(['i', 'plot_id']).size() + dup = dup[dup > 1] + assert len(dup) == 0, ( + f'Malawi/{wave} {rel}: {len(dup)} (i, plot_id) groups hold more than ' + f'one row, so `.first()` is now assembling a skipna COMPOSITE. Check ' + f'whether those rows are the same plot or DIFFERENT plots colliding on ' + f'one id — if different, the identifier is broken and must be fixed ' + f'there (GH #323 D1), not with skipna=False.\n{dup.head()}' + ) + + +@pytest.mark.parametrize('spec', sorted(MODULE_C)) +def test_rows_with_no_plot_id_carry_no_plot_data(spec): + """The 468 rows dropped on a null key are empty stubs, not lost plots. + + `groupby(dropna=True)` deleted these silently; the drop is now an explicit + `dropna(subset=['plot_id'])`. What licenses either is that they measure + NOTHING — no farmer-estimated area, no unit, no GPS area. If a future + release ships an un-keyed row that DOES carry an area, this fails and the + row stops being safe to drop. + """ + wave, rel, idcol, keycols, prefix = spec + expected_rows, expected_null = MODULE_C[spec] + df = _source(wave, rel, convert_categoricals=False) + assert len(df) == expected_rows, ( + f'Malawi/{wave} {rel}: expected {expected_rows} rows, got {len(df)}' + ) + null = _plotkey(df, keycols).isna() + assert int(null.sum()) == expected_null, ( + f'Malawi/{wave} {rel}: expected {expected_null} rows with no plot id, ' + f'got {int(null.sum())}' + ) + areas = [c for c in AREA_COLS if c in df.columns] + if expected_null and areas: + carrying = int(df.loc[null, areas].notna().any(axis=1).sum()) + assert carrying == 0, ( + f'Malawi/{wave} {rel}: {carrying} of the {expected_null} rows with ' + f'no plot id now carry an area measurement. They are no longer ' + f'empty roster stubs, and dropping them loses real data.' + ) + + +def test_plot_features_index_is_unique_and_fully_keyed(malawi): + df = _table(malawi, 'plot_features') + assert df.index.is_unique, 'plot_features (t, i, plot_id) is not unique' + plot_id = df.reset_index()['plot_id'] + assert plot_id.notna().all(), ( + f'{int(plot_id.isna().sum())} plot_features rows have a NULL plot_id; ' + f'they would be deleted unannounced by the groupby that follows' + ) + + +# --------------------------------------------------------------------------- +# Sites 2 and 3 — the two Module H tables. +# --------------------------------------------------------------------------- + +@pytest.mark.parametrize('wave', sorted(MODULE_H)) +def test_module_h_is_one_row_per_household(wave): + """The premise under both Module-H `.first()` sites. + + Also pins that `format_id` merges no two distinct ids: if it did, the + unique count would fall below the raw one and `(t, i)` would stop + identifying a household. + """ + rel, idcol, expected = MODULE_H[wave] + df = _source(wave, rel, convert_categoricals=False) + raw = df[idcol].astype(str) + fid = df[idcol].apply(format_id).astype(str) + assert len(df) == raw.nunique() == expected, ( + f'Malawi/{wave} Module H: expected {expected} rows, one per household; ' + f'got {len(df)} rows / {raw.nunique()} distinct {idcol}' + ) + assert fid.nunique() == raw.nunique(), ( + f'Malawi/{wave}: format_id merges ' + f'{raw.nunique() - fid.nunique()} distinct {idcol} values, so `i` no ' + f'longer distinguishes households' + ) + + +# --------------------------------------------------------------------------- +# Trap 3 — the Tanzania inversion. A key from the WRONG NAMESPACE produces +# ZERO duplicates, so every assertion above can pass on a thoroughly broken +# key. This is the check that distinguishes them. +# --------------------------------------------------------------------------- + +# Households per wave that answered each module. Below the roster's count +# wherever the module was not administered to the whole sample — 2016-17 and +# 2019-20 build the cross-sectional half only. +EXPECTED_HOUSEHOLDS = { + 'plot_features': {'2010-11': 10126, '2013-14': 3061, + '2016-17': 11536, '2019-20': 11135}, + 'food_coping': {'2010-11': 12271, '2013-14': 3999, + '2016-17': 12447, '2019-20': 11432}, + 'months_food_inadequate': {'2010-11': 12271, '2013-14': 4000, + '2016-17': 12447, '2019-20': 11434}, +} + + +@pytest.mark.parametrize('table', sorted(EXPECTED_HOUSEHOLDS)) +def test_i_lives_in_the_same_namespace_as_the_roster(malawi, table): + """PER WAVE, never in aggregate. + + Aggregate is the trap inside the trap: in Tanzania's case two clean waves + carried an aggregate check over four whose overlap was 0. So this asserts + per wave, and asserts the household COUNT too — a wrong-namespace key + inflates the row count without changing the number of underlying facts, and + that inflation is invisible to any duplicate-based instrument. + """ + roster = _table(malawi, 'household_roster').reset_index() + ref = {str(t): set(g['i'].astype(str)) + for t, g in roster.groupby(roster['t'].astype(str))} + + flat = _table(malawi, table).reset_index() + seen = set() + for t, g in flat.groupby(flat['t'].astype(str)): + seen.add(t) + ids = set(g['i'].astype(str)) + known = ref.get(t, set()) + missing = ids - known + assert not missing, ( + f'Malawi {table} t={t}: {len(missing)} of {len(ids)} household ids ' + f'do not appear in household_roster for the SAME wave — `i` is ' + f'built from a different namespace than the roster\'s, which no ' + f'duplicate count can detect. Examples: {sorted(missing)[:5]}' + ) + expected = EXPECTED_HOUSEHOLDS[table].get(t) + if expected is not None: + assert len(ids) == expected, ( + f'Malawi {table} t={t}: {len(ids)} distinct households, ' + f'expected {expected}' + ) + assert seen == set(EXPECTED_HOUSEHOLDS[table]), ( + f'Malawi {table}: waves changed — expected ' + f'{sorted(EXPECTED_HOUSEHOLDS[table])}, got {sorted(seen)}' + ) + + +@pytest.mark.parametrize('table,levels', [ + ('food_coping', ['t', 'i', 'Strategy']), + ('months_food_inadequate', ['t', 'i']), +]) +def test_module_h_tables_are_unique_on_their_declared_index(malawi, table, levels): + """`.first()` collapses nothing at either site — assert the consequence.""" + df = _table(malawi, table) + flat = df.reset_index() + have = [c for c in levels if c in flat.columns] + assert have == levels, ( + f'{table} lost an index level: expected {levels}, have {have}' + ) + dup = flat.groupby(levels, dropna=False).size() + dup = dup[dup > 1] + assert len(dup) == 0, ( + f'Malawi {table}: {len(dup)} {tuple(levels)} groups hold more than one ' + f'row. `.first()` is now assembling a skipna composite; find out ' + f'whether those rows are one household or two before touching the ' + f'reducer.\n{dup.head()}' + ) + assert flat['i'].notna().all(), ( + f'Malawi {table}: {int(flat["i"].isna().sum())} rows have a NULL ' + f'household id, which groupby(dropna=True) deletes outright' + ) From 7bfd5714cb29697634e9dba0f4f2008e3e5a3102 Mon Sep 17 00:00:00 2001 From: Ethan Ligon Date: Tue, 21 Jul 2026 20:48:13 -0700 Subject: [PATCH 3/3] test: load `requires_s3` by PATH, not by import CI's `unit-tests` job collects `tests/` as a PACKAGE, so `from tests.conftest import requires_s3` resolves as `tests.tests.conftest` and raises ModuleNotFoundError; the fallback then hits the repo ROOT conftest.py, which has no `requires_s3`. Both new modules errored at collection. Locally the same line worked, because pytest was inserting the rootdir instead -- so the local run could not have caught it, and did not. Loading the sibling file by path is the same in every import mode. Verified all three: default rootdir-insert, `--import-mode=importlib`, and a whole-`tests/` collection (1,794 tests) -- 38 collected, 0 errors, in each. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/test_gh323_malawi_gb_cartesian.py | 21 +++++++++++++++------ tests/test_gh637_malawi_first_sites.py | 19 +++++++++++++------ 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/tests/test_gh323_malawi_gb_cartesian.py b/tests/test_gh323_malawi_gb_cartesian.py index 8d682efc0..a47a29ef8 100644 --- a/tests/test_gh323_malawi_gb_cartesian.py +++ b/tests/test_gh323_malawi_gb_cartesian.py @@ -31,16 +31,25 @@ WAVE-level frame, upstream of the collapse, and assert the source invariants that license the re-key rather than merely asserting them in a comment. """ +import importlib.util as _iu +from pathlib import Path as _Path + import pandas as pd import pytest import yaml -try: - # `tests/` is a package and the repo ROOT also has a conftest.py, so a bare - # `from conftest import ...` resolves to the wrong one. - from tests.conftest import requires_s3 -except ImportError: # pragma: no cover - from conftest import requires_s3 +# `requires_s3` lives in tests/conftest.py, and importing it is fiddlier than it +# looks: the repo ROOT also has a conftest.py, so a bare `from conftest import` +# picks up the wrong one, while `from tests.conftest import` resolves as +# `tests.tests.conftest` when pytest imports this module as part of the `tests` +# package (which is what CI does -- it failed there and passed locally). Load +# the sibling file by PATH, which is the same in every import mode. +_conftest = _iu.module_from_spec( + _iu.spec_from_file_location( + 'lsms_tests_conftest', _Path(__file__).with_name('conftest.py'))) +_conftest.__loader__.exec_module(_conftest) +requires_s3 = _conftest.requires_s3 + from lsms_library.country import Country from lsms_library.local_tools import get_dataframe, format_id from lsms_library.paths import countries_root diff --git a/tests/test_gh637_malawi_first_sites.py b/tests/test_gh637_malawi_first_sites.py index 643e31e29..e8ef73208 100644 --- a/tests/test_gh637_malawi_first_sites.py +++ b/tests/test_gh637_malawi_first_sites.py @@ -49,15 +49,22 @@ side, so both are in the raw `case_id` namespace and `i` joins exactly. The test below keeps it that way. """ +import importlib.util as _iu +from pathlib import Path as _Path + import pandas as pd import pytest -try: - # `tests/` is a package and the repo ROOT also has a conftest.py, so a bare - # `from conftest import ...` resolves to the wrong one. - from tests.conftest import requires_s3 -except ImportError: # pragma: no cover - from conftest import requires_s3 +# See tests/test_gh323_malawi_gb_cartesian.py for why this is loaded by path +# rather than imported: the root conftest.py shadows a bare `from conftest`, +# and `from tests.conftest` resolves as `tests.tests.conftest` under CI's +# package import mode. +_conftest = _iu.module_from_spec( + _iu.spec_from_file_location( + 'lsms_tests_conftest', _Path(__file__).with_name('conftest.py'))) +_conftest.__loader__.exec_module(_conftest) +requires_s3 = _conftest.requires_s3 + from lsms_library.country import Country from lsms_library.local_tools import get_dataframe, format_id from lsms_library.paths import countries_root