From 53f8c73e008e75d98057e2026ee4ff112b691013 Mon Sep 17 00:00:00 2001 From: jordan inskeep Date: Wed, 23 Sep 2026 12:56:16 -0400 Subject: [PATCH] Fix county subdivision part lookup and chunk long ucgid requests The pseudo lookup added in 0.6.3 read GeoIDFQ attributes by API name, which fails for "county subdivision" (cousub), and gave every scope county every county subdivision. Pair each parent with the children it shares fields with. Also request plain ucgid lists in chunks of 100: the 498 place/remainder parts (070) in region15 in one URL were dropped by the Census API. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 7 +++++++ morpc_census/api.py | 22 ++++++++++++++++----- morpc_census/geos.py | 17 ++++++++++++---- reference/dev_notes.md | 14 +++++++++++++ tests/test_api.py | 24 ++++++++++++++++++++++ tests/test_geos_hierarchical.py | 35 +++++++++++++++++++++++++++++++++ 6 files changed, 110 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e51bce..d3148bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,13 @@ This project uses [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +## [0.6.4] — 2026-09-23 + +### Fixed + +- **`CensusAPI` for county subdivision place/remainder parts (`070`)** in a county scope. The 0.6.3 pseudo lookup used the API name (`county subdivision`) as the `GeoIDFQ` attribute (`cousub`) and gave every scope county every county subdivision; each county is now paired with its own. `070` is published by `dec/dhc` (2020) and `dec/sf1` (2010/2000), not `dec/pl`. +- **Long `ucgid` lists** from the hierarchical geography lookup are requested in chunks of 100 geographies. A single request for the 498 `070` parts in region15 was dropped by the Census API. + ## [0.6.3] — 2026-09-23 ### Fixed diff --git a/morpc_census/api.py b/morpc_census/api.py index 644a626..3a5ba24 100644 --- a/morpc_census/api.py +++ b/morpc_census/api.py @@ -942,15 +942,27 @@ def _fetch_variables(self, url: str, params: dict) -> pd.DataFrame: f"Fetching {len(variables)} variable(s) in {len(batches)} batch(es)." ) + # A plain ucgid list (from the hierarchical geography lookup) can be too long for one URL, so + # request it in chunks of geographies. A pseudo() predicate is short and is sent as-is. + UCGID_CHUNK_SIZE = 100 + ucgid = params.get('ucgid', '') + if ucgid and not ucgid.startswith('pseudo('): + geoids = ucgid.split(',') + geo_chunks = [{'ucgid': ','.join(geoids[j:j + UCGID_CHUNK_SIZE])} + for j in range(0, len(geoids), UCGID_CHUNK_SIZE)] + else: + geo_chunks = [{}] + frames = [] for i, batch in enumerate(batches, 1): self.logger.info(f"Batch {i}/{len(batches)}: {len(batch)} variable(s).") batch_params = {**params, 'get': ','.join(['GEO_ID', 'NAME'] + batch)} - records = get_json_safely(url, params=batch_params) - columns = records.pop(0) - frames.append( - pd.DataFrame.from_records(records, columns=columns).set_index(['GEO_ID', 'NAME']) - ) + chunk_frames = [] + for geo_chunk in geo_chunks: + records = get_json_safely(url, params={**batch_params, **geo_chunk}) + columns = records.pop(0) + chunk_frames.append(pd.DataFrame.from_records(records, columns=columns)) + frames.append(pd.concat(chunk_frames).set_index(['GEO_ID', 'NAME'])) result = frames[0] if len(frames) == 1 else frames[0].join(frames[1:]) return result.reset_index() diff --git a/morpc_census/geos.py b/morpc_census/geos.py index 307ae86..0593342 100644 --- a/morpc_census/geos.py +++ b/morpc_census/geos.py @@ -573,11 +573,20 @@ def geoinfo_for_hierarchical_geos(scope: str | Scope, sumlevel: str | SumLevel) try: # One pseudo query finds only the geographies of this type that intersect the scope. - pseudos = pseudos_from_scope_sumlevel(SumLevel(geo), sc) + child = SumLevel(geo) + pseudos = pseudos_from_scope_sumlevel(child, sc) found = geoinfo_from_params({'ucgid': f"pseudo({','.join(pseudos)})"}, output='table') - geoids = sorted({getattr(GeoIDFQ.parse(fq), geo) for fq in found['GEO_ID']}) - for i in parent_table.index: - parent_table.at[i, geo] = geoids + found = [GeoIDFQ.parse(fq) for fq in found['GEO_ID']] + # GeoIDFQ names components differently from the API (e.g. "cousub" for "county subdivision"). + part = child.parts[-1] + # Give each parent row only the children it contains, e.g. a county's own county subdivisions. + # Places do not nest in counties, so they are matched on state alone. + shared = [x for x in in_scope if SumLevel(x).parts[-1] in child.parts] + for i, row in parent_table.iterrows(): + parent_table.at[i, geo] = sorted({ + getattr(f, part) for f in found + if all(getattr(f, SumLevel(x).parts[-1]) == row[x] for x in shared) + }) except (ValueError, KeyError): for i, row in parent_table.iterrows(): in_param_str = [ diff --git a/reference/dev_notes.md b/reference/dev_notes.md index a038d98..2fb1357 100644 --- a/reference/dev_notes.md +++ b/reference/dev_notes.md @@ -1092,3 +1092,17 @@ Tests: `TestDimensionTableDescriptionTable` replaced by `TestDimensionTableParse **Limitation**: the place list comes from the 2024 geoinfo, so places that no longer exist (e.g. Hidden Lakes CDP, 2020) are not returned. Tests: `tests/test_geos_hierarchical.py` — 3 new tests (scope filtering, one request per place, fallback path). 340 passing. Verified live for 2000/2010/2020 dec/pl: Columbus parts sum to the place total (905,748 in 2020); every other part/place mismatch but Hidden Lakes is an out-of-region county part. + +## 2026-09-23 — Fix hierarchical lookup for county subdivision parts (070) and chunk long ucgid lists (branch fix/hierarchical-cousub-parts) + +**Bug 1**: The 0.6.3 pseudo lookup in `geoinfo_for_hierarchical_geos()` read `getattr(GeoIDFQ, geo)` with the API name, which fails for `county subdivision` (GeoIDFQ calls it `cousub`), and assigned the same child list to every parent row. That is right for places (they do not nest in counties) but pairs every scope county with every county subdivision for 070. + +**Fix**: Use `SumLevel(geo).parts[-1]` for the attribute, and give each parent row only the children that match it on the fields they share (`county` for county subdivisions; only `state` for places). + +**Bug 2**: `CensusAPI._fetch_variables()` sent the hierarchical ucgid list in one request. For region15 070 that is 498 GEOIDs (~12 KB), and the Census API drops the connection. + +**Fix**: Request plain ucgid lists in chunks of 100; pseudo() predicates are unchanged. + +**Note**: `dec/pl` does not publish 070 in any year; `dec/dhc` (2020) and `dec/sf1` (2010/2000) do. Their county subdivision totals match `dec/pl` except where parts of places that no longer exist are missing from the current-geography lookup (e.g. Hidden Lakes CDP). + +Tests: 1 new in `tests/test_geos_hierarchical.py`, 2 new in `tests/test_api.py`. 343 passing. Live: region15 070 returns 496 / 481 / 421 parts for 2020 / 2010 / 2000, covering all 237 MORPC-lookup 070 geographies each year (~145 s per call). diff --git a/tests/test_api.py b/tests/test_api.py index c9b5ead..7a23dfe 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -1084,6 +1084,30 @@ def test_single_row_result(self): result = api._fetch_variables(api.request['url'], {}) assert len(result) == 1 + def test_long_ucgid_list_requested_in_chunks(self): + # The hierarchical geography lookup returns a plain ucgid list (e.g. 498 place/remainder parts), + # which is too long for one Census API URL. + api = self._make_api(1) + geoids = [f'0700000US39049{i:05d}99999' for i in range(250)] + + def respond(url, params): + chunk = params['ucgid'].split(',') + return [['GEO_ID', 'NAME'] + api.variables] + [[g, g] + ['1'] for g in chunk] + + with patch('morpc.req.get_json_safely', side_effect=respond) as mock: + result = api._fetch_variables(api.request['url'], {'ucgid': ','.join(geoids)}) + assert mock.call_count == 3 + assert all(len(c.kwargs['params']['ucgid'].split(',')) <= 100 for c in mock.call_args_list) + assert sorted(result['GEO_ID']) == geoids + + def test_pseudo_ucgid_is_not_chunked(self): + api = self._make_api(1) + ucgid = 'pseudo(0500000US39049$1400000)' + with patch('morpc.req.get_json_safely', return_value=self._response(api.variables)) as mock: + api._fetch_variables(api.request['url'], {'ucgid': ucgid}) + mock.assert_called_once() + assert mock.call_args.kwargs['params']['ucgid'] == ucgid + class TestFetchDispatch: """Tests for _fetch choosing between the group() and variable-list paths.""" diff --git a/tests/test_geos_hierarchical.py b/tests/test_geos_hierarchical.py index 8633654..bde2a15 100644 --- a/tests/test_geos_hierarchical.py +++ b/tests/test_geos_hierarchical.py @@ -76,3 +76,38 @@ def test_fallback_without_pseudo_also_queries_single_places(): assert all("," not in p for p in places) assert len(places) == len(set(places)) assert sorted(result["GEO_ID"]) == ["1550000US3902582041", "1550000US3918000041", "1550000US3918000049"] + + +# County subdivision place/remainder parts (070) require state, county, and county subdivision. The county is in +# the scope, so each county must be paired only with its own county subdivisions, which the API names +# "county subdivision" but GeoIDFQ calls "cousub". + +COUSUBS = {"041": ["02582", "04920"], "049": ["18000"]} + + +def _fake_cousub_geoinfo(calls): + def fake(param_dict, *args, **kwargs): + calls.append(param_dict) + if "ucgid" in param_dict: + geoids = [f"0600000US39{county}{cousub}" for county, cousubs in COUSUBS.items() for cousub in cousubs] + return pd.DataFrame({"GEO_ID": geoids, "NAME": geoids, "ucgid": geoids}) + parts = dict(p.split(":") for p in param_dict["in"]) + geoid = f"0700000US39{parts['county']}{parts['county subdivision']}99999" + return pd.DataFrame({"GEO_ID": [geoid], "NAME": [geoid]}) + return fake + + +def test_county_subdivision_parts_pair_each_county_with_its_own_subdivisions(): + calls = [] + with patch.object(SumLevel, "get_query_req", return_value={"requires": ["state", "county", "county subdivision"], "wildcard": None}), \ + patch("morpc_census.geos.geoids_from_scope", return_value=SCOPE_TABLE), \ + patch("morpc_census.geos.geoinfo_from_params", side_effect=_fake_cousub_geoinfo(calls)), \ + patch("morpc_census.geos.pseudos_from_scope_sumlevel", return_value=["x"]): + result = geoinfo_for_hierarchical_geos(SCOPE, SumLevel("070")) + requests = sorted(tuple(sorted(c["in"])) for c in calls if "for" in c) + assert requests == [ + ("county subdivision:02582", "county:041", "state:39"), + ("county subdivision:04920", "county:041", "state:39"), + ("county subdivision:18000", "county:049", "state:39"), + ] + assert len(result) == 3