From acfe92b827dc33544921e0dc44fbae01b98c2702 Mon Sep 17 00:00:00 2001 From: jordan inskeep Date: Wed, 30 Sep 2026 11:38:37 -0400 Subject: [PATCH 1/3] Chunk long ucgid lists in the group() fetch path _fetch_group sent the full hierarchical ucgid list in one URL, so large requests (e.g. region15 at sumlevel 070, ~15k chars) failed with RemoteDisconnected. Move the chunking from _fetch_variables into a shared _ucgid_chunks helper and use it in both fetch paths. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 4 ++++ morpc_census/api.py | 50 ++++++++++++++++++++++++------------------ reference/dev_notes.md | 10 +++++++++ tests/test_api.py | 46 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 89 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5dc6d45..9b7fd52 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ This project uses [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Fixed + +- **`CensusAPI` with `group=` now requests long geography lists in chunks of 100.** `_fetch_group()` sent the full ucgid list from the hierarchical geography lookup in one URL, so a request such as region15 at sumlevel `070` (~15k characters) failed with `ConnectionError: RemoteDisconnected`. `_fetch_variables()` already chunked; both now share the same helper. `pseudo()` predicates are unchanged. + ## [0.6.5] — 2026-09-24 ### Security diff --git a/morpc_census/api.py b/morpc_census/api.py index 373e8f5..caf7ada 100644 --- a/morpc_census/api.py +++ b/morpc_census/api.py @@ -667,6 +667,20 @@ def find_replace_variable_map(labels: list[str], variables: list[str], label_map return new_labels, new_variables +def _ucgid_chunks(params: dict, chunk_size: int = 100) -> list[dict]: + """Split a plain ucgid list into chunks of geographies, returned as param overrides. + + A plain ucgid list (from the hierarchical geography lookup) can be too long for one URL; + the Census API drops the connection past roughly 8k characters. A pseudo() predicate is + short and is sent as-is, as is a request with no ucgid. + """ + ucgid = params.get('ucgid', '') + if not ucgid or ucgid.startswith('pseudo('): + return [{}] + geoids = ucgid.split(',') + return [{'ucgid': ','.join(geoids[j:j + chunk_size])} for j in range(0, len(geoids), chunk_size)] + + # --------------------------------------------------------------------------- # CensusAPI # --------------------------------------------------------------------------- @@ -914,17 +928,20 @@ def _fetch_group(self, url: str, params: dict) -> pd.DataFrame: from morpc.req import get_text_safely self.logger.info(f"Fetching group({self.group.code}) — all variables, no limit.") - params_string = "&".join(f"{k}={v}" for k, v in params.items()) - text = get_text_safely(f"{url}{params_string}") - try: - df = pd.read_csv( - StringIO(text.replace('[', '').replace(']', '').rstrip(',')), - sep=',', quotechar='"', - ) - return df.drop(columns=[c for c in df.columns if c.startswith('Unnamed')]) - except Exception as e: - self.logger.error(f"Failed to parse group response: {e}") - raise RuntimeError("Failed to parse Census API group response.") from e + frames = [] + for geo_chunk in _ucgid_chunks(params): + params_string = "&".join(f"{k}={v}" for k, v in {**params, **geo_chunk}.items()) + text = get_text_safely(f"{url}{params_string}") + try: + df = pd.read_csv( + StringIO(text.replace('[', '').replace(']', '').rstrip(',')), + sep=',', quotechar='"', + ) + except Exception as e: + self.logger.error(f"Failed to parse group response: {e}") + raise RuntimeError("Failed to parse Census API group response.") from e + frames.append(df.drop(columns=[c for c in df.columns if c.startswith('Unnamed')])) + return pd.concat(frames, ignore_index=True) def _fetch_variables(self, url: str, params: dict) -> pd.DataFrame: """Fetch a specific variable list, batching into chunks of 49. @@ -943,16 +960,7 @@ 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 = [{}] + geo_chunks = _ucgid_chunks(params) frames = [] for i, batch in enumerate(batches, 1): diff --git a/reference/dev_notes.md b/reference/dev_notes.md index c34e3a6..290a0fe 100644 --- a/reference/dev_notes.md +++ b/reference/dev_notes.md @@ -1114,3 +1114,13 @@ Tests: 1 new in `tests/test_geos_hierarchical.py`, 2 new in `tests/test_api.py`. **API key**: `geoinfo_from_params()` logged its params at INFO after adding `key` (#7, fixed in #8). morpc 0.7.5 redacts `key`/`token`/`api_key` in all `morpc.req` logs and errors (morpc/morpc-py#207); morpc-census now requires it. Tests: 3 in `tests/test_api.py` (204 chunk skipped, all-204 returns empty frame, other errors raise) and 1 in `tests/test_geos_hierarchical.py` (key not logged). 347 passing. + +## 2026-09-30 — Chunk long ucgid lists in the group() fetch path (branch fix/group-fetch-ucgid-chunking) + +**Bug**: The 2026-09-23 fix chunked plain ucgid lists only in `CensusAPI._fetch_variables()`. `_fetch_group()` (used when only `group=` is given) still sent the whole list in one URL. For region15 at 070 that is ~15k characters; the Census API drops the connection with `RemoteDisconnected` above roughly 8k (5.8k succeeded, 11.5k failed in a live test). + +**Fix**: Moved the chunking into a module helper `_ucgid_chunks()` in `api.py`, used by both fetch paths. `_fetch_group()` fetches and parses each chunk and concatenates the frames. + +**Note**: `_fetch_group()` still does not treat 204 No Content as no rows (only `_fetch_variables()` does), so a chunk with no data raises `HTTPError`. + +Tests: 2 new in `tests/test_api.py` (`TestFetchGroupChunking`: long list split into ≤100 per request, pseudo not chunked). 349 passing. Live: a 627-GEOID `group(P1)` request against 2020 `dec/dhc` (the URL that previously disconnected) returns data in 7 requests. diff --git a/tests/test_api.py b/tests/test_api.py index 6fcfae6..2d69e3d 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -1148,6 +1148,52 @@ def test_other_http_errors_still_raise(self): api._fetch_variables(api.request['url'], {'ucgid': '0700000US390491800099999'}) +class TestFetchGroupChunking: + """Tests for _fetch_group splitting long ucgid lists across requests.""" + + _fake_endpoints = {'acs/acs5': [2023]} + + def _make_api(self): + """Build a CensusAPI with _fetch stubbed so we can call _fetch_group directly.""" + stub = pd.DataFrame({'GEO_ID': ['0500000US39049'], 'NAME': ['Franklin County']}) + with patch('morpc_census.api.get_all_avail_endpoints', return_value=self._fake_endpoints), \ + patch('morpc_census.geos.geoinfo_from_scope_sumlevel', return_value={'for': 'county:049'}), \ + patch.object(CensusAPI, '_fetch', return_value=stub): + return CensusAPI(Endpoint('acs/acs5', 2023), 'franklin', group='B01001', return_long=False) + + @staticmethod + def _respond(url): + """Simulate the group() text response: one row per geography in the URL's ucgid.""" + chunk = url.split('ucgid=')[1].split('&')[0].split(',') + rows = ['["GEO_ID","NAME","B01001_001E",'] + [f'["{g}","{g}","1"],' for g in chunk] + return '[' + '\n'.join(rows) + ']' + + def test_long_ucgid_list_requested_in_chunks(self): + # A region-wide 070 lookup yields hundreds of geographies; one URL that long is + # dropped by the Census API with RemoteDisconnected. + api = self._make_api() + geoids = [f'0700000US39049{i:05d}99999' for i in range(250)] + params = {'get': 'group(B01001)', 'ucgid': ','.join(geoids)} + with patch('morpc.req.get_text_safely', side_effect=self._respond) as mock: + result = api._fetch_group('https://api.census.gov/data/2023/acs/acs5?', params) + assert mock.call_count == 3 + for call in mock.call_args_list: + assert len(call.args[0].split('ucgid=')[1].split('&')[0].split(',')) <= 100 + assert 'get=group(B01001)' in call.args[0] + assert sorted(result['GEO_ID']) == geoids + assert list(result.index) == list(range(250)) + + def test_pseudo_ucgid_is_not_chunked(self): + api = self._make_api() + params = {'get': 'group(B01001)', 'ucgid': 'pseudo(0500000US39049$1400000)'} + text = '[["GEO_ID","NAME","B01001_001E"],\n["1400000US39049000100","Tract 1","1"]]' + with patch('morpc.req.get_text_safely', return_value=text) as mock: + result = api._fetch_group('u?', params) + mock.assert_called_once() + assert 'ucgid=pseudo(0500000US39049$1400000)' in mock.call_args.args[0] + assert len(result) == 1 + + class TestFetchDispatch: """Tests for _fetch choosing between the group() and variable-list paths.""" From f56ae38abdd88bced5c0a4a676b9b685b56d2901 Mon Sep 17 00:00:00 2001 From: jordan inskeep Date: Wed, 30 Sep 2026 11:42:43 -0400 Subject: [PATCH 2/3] Treat 204 No Content as no rows in the group() fetch path _fetch_group raised HTTPError on 204, so group= requests for geographies without data (e.g. dec/pl at sumlevel 070) failed, and with ucgid chunking a single empty chunk would stop the whole fetch. Skip 204 chunks with a warning, as _fetch_variables does, and return an empty frame with the group's columns when every chunk is empty. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + morpc_census/api.py | 13 ++++++++++++- reference/dev_notes.md | 4 ++-- tests/test_api.py | 37 +++++++++++++++++++++++++++++++++++++ 4 files changed, 52 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b7fd52..349aa41 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ This project uses [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixed - **`CensusAPI` with `group=` now requests long geography lists in chunks of 100.** `_fetch_group()` sent the full ucgid list from the hierarchical geography lookup in one URL, so a request such as region15 at sumlevel `070` (~15k characters) failed with `ConnectionError: RemoteDisconnected`. `_fetch_variables()` already chunked; both now share the same helper. `pseudo()` predicates are unchanged. +- **A 204 No Content response is treated as no rows for `group=` requests too**, matching the variable-list path since 0.6.5. For example, `CensusAPI(Endpoint('dec/pl', 2020), 'franklin', group='P1', sumlevel='070')` now returns an empty `long` with a warning instead of raising `HTTPError`. Other HTTP errors still raise. ## [0.6.5] — 2026-09-24 diff --git a/morpc_census/api.py b/morpc_census/api.py index caf7ada..191f600 100644 --- a/morpc_census/api.py +++ b/morpc_census/api.py @@ -926,12 +926,21 @@ def _fetch_group(self, url: str, params: dict) -> pd.DataFrame: but the response is a flat text stream rather than JSON. """ from morpc.req import get_text_safely + from requests import HTTPError self.logger.info(f"Fetching group({self.group.code}) — all variables, no limit.") frames = [] for geo_chunk in _ucgid_chunks(params): params_string = "&".join(f"{k}={v}" for k, v in {**params, **geo_chunk}.items()) - text = get_text_safely(f"{url}{params_string}") + try: + text = get_text_safely(f"{url}{params_string}") + except HTTPError as e: + # 204 No Content: the request is valid but Census has no data for these geographies + # (e.g. dec/pl publishes no county subdivision parts, 070). + if e.response is None or e.response.status_code != 204: + raise + self.logger.warning(f"Census API returned no data for {self.name}; treating the request as no rows.") + continue try: df = pd.read_csv( StringIO(text.replace('[', '').replace(']', '').rstrip(',')), @@ -941,6 +950,8 @@ def _fetch_group(self, url: str, params: dict) -> pd.DataFrame: self.logger.error(f"Failed to parse group response: {e}") raise RuntimeError("Failed to parse Census API group response.") from e frames.append(df.drop(columns=[c for c in df.columns if c.startswith('Unnamed')])) + if not frames: + return pd.DataFrame(columns=['GEO_ID', 'NAME'] + list(self.vars)) return pd.concat(frames, ignore_index=True) def _fetch_variables(self, url: str, params: dict) -> pd.DataFrame: diff --git a/reference/dev_notes.md b/reference/dev_notes.md index 290a0fe..59938d8 100644 --- a/reference/dev_notes.md +++ b/reference/dev_notes.md @@ -1121,6 +1121,6 @@ Tests: 3 in `tests/test_api.py` (204 chunk skipped, all-204 returns empty frame, **Fix**: Moved the chunking into a module helper `_ucgid_chunks()` in `api.py`, used by both fetch paths. `_fetch_group()` fetches and parses each chunk and concatenates the frames. -**Note**: `_fetch_group()` still does not treat 204 No Content as no rows (only `_fetch_variables()` does), so a chunk with no data raises `HTTPError`. +**204**: `_fetch_group()` did not treat 204 No Content as no rows (only `_fetch_variables()` did), so `group=` requests with no data (e.g. `dec/pl` at 070) raised `HTTPError`, and with chunking one empty chunk would stop the whole fetch. It now skips 204 chunks with a warning, like `_fetch_variables()`; if every chunk is empty it returns an empty frame with `GEO_ID`, `NAME` and the group's variables, so `CensusAPI.long` is empty. Other HTTP errors still raise. -Tests: 2 new in `tests/test_api.py` (`TestFetchGroupChunking`: long list split into ≤100 per request, pseudo not chunked). 349 passing. Live: a 627-GEOID `group(P1)` request against 2020 `dec/dhc` (the URL that previously disconnected) returns data in 7 requests. +Tests: 5 new in `tests/test_api.py` (`TestFetchGroupChunking`: long list split into ≤100 per request, pseudo not chunked, 204 chunk skipped, all-204 returns empty frame, other errors raise). 352 passing. Live: 2020 `dec/pl` P1 for Franklin at 070 now returns an empty `long` instead of raising. Live: a 627-GEOID `group(P1)` request against 2020 `dec/dhc` (the URL that previously disconnected) returns data in 7 requests. diff --git a/tests/test_api.py b/tests/test_api.py index 2d69e3d..19dca48 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -1193,6 +1193,43 @@ def test_pseudo_ucgid_is_not_chunked(self): assert 'ucgid=pseudo(0500000US39049$1400000)' in mock.call_args.args[0] assert len(result) == 1 + @staticmethod + def _http_error(status): + from requests import HTTPError, Response + response = Response() + response.status_code = status + return HTTPError(f"Request failed with status {status}", response=response) + + def test_no_content_chunk_is_treated_as_no_rows(self): + # The Census API answers 204 when it has no data for the requested geographies, + # e.g. dec/pl for county subdivision parts (070). + api = self._make_api() + geoids = [f'0700000US39049{i:05d}99999' for i in range(150)] + + def respond(url): + if url.split('ucgid=')[1].startswith(geoids[100]): + raise self._http_error(204) + return self._respond(url) + + with patch('morpc.req.get_text_safely', side_effect=respond): + result = api._fetch_group('u?', {'get': 'group(B01001)', 'ucgid': ','.join(geoids)}) + assert sorted(result['GEO_ID']) == geoids[:100] + + def test_no_content_for_every_chunk_returns_empty_frame(self): + api = self._make_api() + with patch.object(Group, 'variables', new={'B01001_001E': {}, 'B01001_002E': {}}), \ + patch('morpc.req.get_text_safely', side_effect=self._http_error(204)): + result = api._fetch_group('u?', {'get': 'group(B01001)', 'ucgid': '0700000US390491800099999'}) + assert result.empty + assert list(result.columns) == ['GEO_ID', 'NAME', 'B01001_001E', 'B01001_002E'] + + def test_other_http_errors_still_raise(self): + from requests import HTTPError + api = self._make_api() + with patch('morpc.req.get_text_safely', side_effect=self._http_error(400)): + with pytest.raises(HTTPError): + api._fetch_group('u?', {'get': 'group(B01001)', 'ucgid': '0700000US390491800099999'}) + class TestFetchDispatch: """Tests for _fetch choosing between the group() and variable-list paths.""" From 044bfe6f40f446a3c3fcda09bffff5e22adc59c9 Mon Sep 17 00:00:00 2001 From: jordan inskeep Date: Wed, 30 Sep 2026 11:45:11 -0400 Subject: [PATCH 3/3] Document 0.6.6 Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 349aa41..ed58dd8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ This project uses [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +## [0.6.6] — 2026-09-30 + ### Fixed - **`CensusAPI` with `group=` now requests long geography lists in chunks of 100.** `_fetch_group()` sent the full ucgid list from the hierarchical geography lookup in one URL, so a request such as region15 at sumlevel `070` (~15k characters) failed with `ConnectionError: RemoteDisconnected`. `_fetch_variables()` already chunked; both now share the same helper. `pseudo()` predicates are unchanged.