Chunk long ucgid lists and handle 204 in group() fetch (v0.6.6)

CHANGELOG.md

@@ -9,6 +9,13 @@ 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.
+- **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
 
 ### Security

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
 # ---------------------------------------------------------------------------
@@ -912,19 +926,33 @@ 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.")
-        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())
+            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(',')),
+                    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')]))
+        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:
         """Fetch a specific variable list, batching into chunks of 49.
@@ -943,16 +971,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):

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.
+
+**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: 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.

tests/test_api.py

@@ -1148,6 +1148,89 @@ 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
+
+    @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."""