diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f0e790..80b6fc7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,32 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [1.9.0] - 2026-08-11 + +### Added + +- `Mappings.get()` / `AsyncMappings.get()` accept `page` and `page_size`. + `GET /v1/concepts/{id}/mappings` became paginated; before that + it applied a fixed `LIMIT 100` server-side with no total and no `has_next`, so + a concept with 1,500 mappings returned 100 of them and nothing in the response + said so. `page_size` defaults to 100, matching the old cap, so an existing + call returns exactly the page it returned before. +- `Mappings.get_iter()` / `AsyncMappings.get_iter()` walk every page and yield + each mapping. Prefer these when building a code list: `get()` returns the + `data` field only, so the `meta.pagination` that would tell you the set was + truncated is not part of what you get back. + +### Fixed + +- `include_invalid=False` now reaches the server on `Mappings.get()` / + `get_iter()` and their async counterparts. The parameter was only sent when + truthy, and this endpoint defaults to *including* deprecated mappings, so + asking to exclude them did nothing. It is now `bool | None`: omit it for the + server default, pass `False` to exclude. Omitting it behaves exactly as + before, so only callers who explicitly passed `False` - and were being + ignored. + + ## [1.8.1] - 2026-06-01 ### Changed @@ -22,7 +48,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `FhirResolution` now types the `value_as_concept` and `value_target_field` fields the resolver returns when a composite concept is decomposed via the - `Maps to value` relationship (HL7 FHIR-to-OMOP IG Value-as-Concept pattern — + `Maps to value` relationship (HL7 FHIR-to-OMOP IG Value-as-Concept pattern - e.g. "Allergy to penicillin" → standard "Allergy to drug" + value "Penicillin G"), plus `concept_map_id` / `mapping_note` for FHIR administrative-code resolutions. These were already passed through; they are @@ -38,7 +64,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [1.7.1] - 2026-05-20 -Maintenance release. Dependency and lock-file updates only — there are no +Maintenance release. Dependency and lock-file updates only - there are no source code or public API changes. The published runtime dependencies (`httpx`, `typing_extensions`) are unchanged; the updates below affect the pinned development, testing, and optional-extra dependencies in `uv.lock` @@ -49,12 +75,12 @@ and are not shipped in the wheel/sdist. - Updated pinned development and transitive dependencies in `uv.lock` to resolve reported advisories. None of these affect the published runtime dependencies: - - `idna` 3.11 → 3.15 — fixes a bypass of the CVE-2024-3651 mitigation in + - `idna` 3.11 → 3.15 - fixes a bypass of the CVE-2024-3651 mitigation in `idna.encode()` (transitive via `httpx`/`anyio`). - - `pytest` 9.0.2 → 9.0.3 — fixes vulnerable tmpdir handling (dev only). - - `python-dotenv` 1.2.1 → 1.2.2 — fixes symlink following in `set_key` + - `pytest` 9.0.2 → 9.0.3 - fixes vulnerable tmpdir handling (dev only). + - `python-dotenv` 1.2.1 → 1.2.2 - fixes symlink following in `set_key` that allowed arbitrary file overwrite (dev only). - - `pygments` 2.19.2 → 2.20.0 — fixes a ReDoS in GUID matching + - `pygments` 2.19.2 → 2.20.0 - fixes a ReDoS in GUID matching (transitive, dev only). ### Changed @@ -275,7 +301,8 @@ and are not shipped in the wheel/sdist. - Full type hints and PEP 561 compliance - HTTP/2 support via httpx -[Unreleased]: https://github.com/omopHub/omophub-python/compare/v1.8.1...HEAD +[Unreleased]: https://github.com/omopHub/omophub-python/compare/v1.9.0...HEAD +[1.9.0]: https://github.com/omopHub/omophub-python/compare/v1.8.1...v1.9.0 [1.8.1]: https://github.com/omopHub/omophub-python/compare/v1.8.0...v1.8.1 [1.8.0]: https://github.com/omopHub/omophub-python/compare/v1.7.1...v1.8.0 [1.7.1]: https://github.com/omopHub/omophub-python/compare/v1.7.0...v1.7.1 diff --git a/src/omophub/resources/mappings.py b/src/omophub/resources/mappings.py index 1b3399e..5ce84ba 100644 --- a/src/omophub/resources/mappings.py +++ b/src/omophub/resources/mappings.py @@ -4,8 +4,13 @@ from typing import TYPE_CHECKING, Any +from .._pagination import paginate_async, paginate_sync + if TYPE_CHECKING: + from collections.abc import AsyncIterator, Iterator + from .._request import AsyncRequest, Request + from .._types import PaginationMeta class Mappings: @@ -19,25 +24,43 @@ def get( concept_id: int, *, target_vocabulary: str | None = None, - include_invalid: bool = False, + include_invalid: bool | None = None, + page: int = 1, + page_size: int = 100, vocab_release: str | None = None, ) -> dict[str, Any]: """Get mappings for a concept. + A concept can have more mappings than one page carries, and this + method returns the ``data`` field only — the pagination metadata that + would tell you so is not part of what you get back. Use + :meth:`get_iter` when you need every mapping, and treat a full page here + as "there is probably more" rather than as the complete set. + Args: concept_id: The concept ID target_vocabulary: Filter to a specific target vocabulary (e.g., "ICD10CM") - include_invalid: Include invalid/deprecated mappings + include_invalid: Whether to return mappings whose relationship or + target concept is deprecated. Omit to take the server default, + which for this endpoint is to **include** them; pass ``False`` + to exclude them. The source concept is never filtered, so a + deprecated concept still returns what it maps to. + page: Page number, 1-based (default 1) + page_size: Mappings per page (default 100). The server clamps this to + 200 on this endpoint and does not report having done so, so a + larger value silently yields a smaller page. vocab_release: Specific vocabulary release version (e.g., "2025.1") Returns: - Mappings for the concept + The response ``data`` field only. ``meta.pagination`` is **not** + part of it, so nothing in this return value tells you whether the + mappings were truncated -- use :meth:`get_iter` when that matters. """ - params: dict[str, Any] = {} + params: dict[str, Any] = {"page": page, "page_size": page_size} if target_vocabulary: params["target_vocabulary"] = target_vocabulary - if include_invalid: - params["include_invalid"] = "true" + if include_invalid is not None: + params["include_invalid"] = "true" if include_invalid else "false" if vocab_release: params["vocab_release"] = vocab_release @@ -45,6 +68,59 @@ def get( f"/concepts/{concept_id}/mappings", params=params or None ) + def get_iter( + self, + concept_id: int, + *, + target_vocabulary: str | None = None, + include_invalid: bool | None = None, + page_size: int = 100, + vocab_release: str | None = None, + ) -> Iterator[dict[str, Any]]: + """Iterate over every mapping for a concept, across all pages. + + Prefer this to :meth:`get` when you are building a code list. A single + page is capped server-side, so a concept with more mappings than the + page size yields a subset that looks exactly like a complete answer. + + Args: + concept_id: The concept ID + target_vocabulary: Filter to a specific target vocabulary (e.g., "ICD10CM") + include_invalid: Whether to return mappings whose relationship or + target concept is deprecated. Omit to take the server default, + which for this endpoint is to **include** them; pass ``False`` + to exclude them. The source concept is never filtered, so a + deprecated concept still returns what it maps to. + page_size: Mappings fetched per request (default 100, server max 200) + vocab_release: Specific vocabulary release version (e.g., "2025.1") + + Yields: + Individual mappings from all pages + """ + + def fetch_page( + page: int, size: int + ) -> tuple[list[dict[str, Any]], PaginationMeta | None]: + params: dict[str, Any] = {"page": page, "page_size": size} + if target_vocabulary: + params["target_vocabulary"] = target_vocabulary + if include_invalid is not None: + params["include_invalid"] = "true" if include_invalid else "false" + if vocab_release: + params["vocab_release"] = vocab_release + + # get_raw() rather than get(): the pagination meta is the entire + # point here and get() discards it. + result = self._request.get_raw( + f"/concepts/{concept_id}/mappings", params=params + ) + data = result.get("data") or {} + mappings = data.get("mappings", []) if isinstance(data, dict) else data + meta = result.get("meta", {}).get("pagination") + return mappings, meta + + yield from paginate_sync(fetch_page, page_size) + def map( self, target_vocabulary: str, @@ -116,25 +192,43 @@ async def get( concept_id: int, *, target_vocabulary: str | None = None, - include_invalid: bool = False, + include_invalid: bool | None = None, + page: int = 1, + page_size: int = 100, vocab_release: str | None = None, ) -> dict[str, Any]: """Get mappings for a concept. + A concept can have more mappings than one page carries, and this + method returns the ``data`` field only — the pagination metadata that + would tell you so is not part of what you get back. Use + :meth:`get_iter` when you need every mapping, and treat a full page here + as "there is probably more" rather than as the complete set. + Args: concept_id: The concept ID target_vocabulary: Filter to a specific target vocabulary (e.g., "ICD10CM") - include_invalid: Include invalid/deprecated mappings + include_invalid: Whether to return mappings whose relationship or + target concept is deprecated. Omit to take the server default, + which for this endpoint is to **include** them; pass ``False`` + to exclude them. The source concept is never filtered, so a + deprecated concept still returns what it maps to. + page: Page number, 1-based (default 1) + page_size: Mappings per page (default 100). The server clamps this to + 200 on this endpoint and does not report having done so, so a + larger value silently yields a smaller page. vocab_release: Specific vocabulary release version (e.g., "2025.1") Returns: - Mappings for the concept + The response ``data`` field only. ``meta.pagination`` is **not** + part of it, so nothing in this return value tells you whether the + mappings were truncated -- use :meth:`get_iter` when that matters. """ - params: dict[str, Any] = {} + params: dict[str, Any] = {"page": page, "page_size": page_size} if target_vocabulary: params["target_vocabulary"] = target_vocabulary - if include_invalid: - params["include_invalid"] = "true" + if include_invalid is not None: + params["include_invalid"] = "true" if include_invalid else "false" if vocab_release: params["vocab_release"] = vocab_release @@ -142,6 +236,60 @@ async def get( f"/concepts/{concept_id}/mappings", params=params or None ) + async def get_iter( + self, + concept_id: int, + *, + target_vocabulary: str | None = None, + include_invalid: bool | None = None, + page_size: int = 100, + vocab_release: str | None = None, + ) -> AsyncIterator[dict[str, Any]]: + """Iterate over every mapping for a concept, across all pages. + + Prefer this to :meth:`get` when you are building a code list. A single + page is capped server-side, so a concept with more mappings than the + page size yields a subset that looks exactly like a complete answer. + + Args: + concept_id: The concept ID + target_vocabulary: Filter to a specific target vocabulary (e.g., "ICD10CM") + include_invalid: Whether to return mappings whose relationship or + target concept is deprecated. Omit to take the server default, + which for this endpoint is to **include** them; pass ``False`` + to exclude them. The source concept is never filtered, so a + deprecated concept still returns what it maps to. + page_size: Mappings fetched per request (default 100, server max 200) + vocab_release: Specific vocabulary release version (e.g., "2025.1") + + Yields: + Individual mappings from all pages + """ + + async def fetch_page( + page: int, size: int + ) -> tuple[list[dict[str, Any]], PaginationMeta | None]: + params: dict[str, Any] = {"page": page, "page_size": size} + if target_vocabulary: + params["target_vocabulary"] = target_vocabulary + if include_invalid is not None: + params["include_invalid"] = "true" if include_invalid else "false" + if vocab_release: + params["vocab_release"] = vocab_release + + # get_raw() rather than get(): the pagination meta is the entire + # point here and get() discards it. + result = await self._request.get_raw( + f"/concepts/{concept_id}/mappings", params=params + ) + data = result.get("data") or {} + mappings = data.get("mappings", []) if isinstance(data, dict) else data + meta = result.get("meta", {}).get("pagination") + return mappings, meta + + async for item in paginate_async(fetch_page, page_size): + yield item + async def map( self, target_vocabulary: str, diff --git a/tests/unit/resources/test_mappings.py b/tests/unit/resources/test_mappings.py index dfdd691..a4cefba 100644 --- a/tests/unit/resources/test_mappings.py +++ b/tests/unit/resources/test_mappings.py @@ -59,6 +59,130 @@ def test_get_mappings_with_filters( assert "target_vocabulary=ICD10CM" in url_str assert "include_invalid=true" in url_str + @respx.mock + def test_get_mappings_include_invalid_is_tri_state( + self, sync_client: OMOPHub, base_url: str + ) -> None: + """False must reach the wire, not be dropped as falsy. + + This endpoint defaults to *including* deprecated mappings, so omitting + the parameter and sending ``false`` mean different things. Sending + nothing for ``include_invalid=False`` silently returned the rows the + caller asked to exclude. + """ + route = respx.get(f"{base_url}/concepts/201826/mappings").mock( + return_value=Response(200, json={"success": True, "data": {"mappings": []}}) + ) + + sync_client.mappings.get(201826) + assert "include_invalid" not in str(route.calls[0].request.url) + + sync_client.mappings.get(201826, include_invalid=False) + assert "include_invalid=false" in str(route.calls[1].request.url) + + sync_client.mappings.get(201826, include_invalid=True) + assert "include_invalid=true" in str(route.calls[2].request.url) + + @respx.mock + def test_get_iter_forwards_include_invalid_false( + self, sync_client: OMOPHub, base_url: str + ) -> None: + """The same tri-state has to survive the pagination helper.""" + route = respx.get(f"{base_url}/concepts/201826/mappings").mock( + return_value=Response( + 200, + json={ + "success": True, + "data": {"mappings": []}, + "meta": { + "pagination": { + "page": 1, + "page_size": 100, + "total_items": 0, + "total_pages": 0, + "has_next": False, + "has_previous": False, + } + }, + }, + ) + ) + + list(sync_client.mappings.get_iter(201826, include_invalid=False)) + assert "include_invalid=false" in str(route.calls[0].request.url) + + @respx.mock + def test_get_mappings_sends_pagination( + self, sync_client: OMOPHub, base_url: str + ) -> None: + """page/page_size reach the wire, including their defaults.""" + route = respx.get(f"{base_url}/concepts/201826/mappings").mock( + return_value=Response(200, json={"success": True, "data": {"mappings": []}}) + ) + + sync_client.mappings.get(201826) + default_url = str(route.calls[0].request.url) + assert "page=1" in default_url + assert "page_size=100" in default_url + + sync_client.mappings.get(201826, page=3, page_size=200) + paged_url = str(route.calls[1].request.url) + assert "page=3" in paged_url + assert "page_size=200" in paged_url + + @respx.mock + def test_get_iter_walks_every_page( + self, sync_client: OMOPHub, base_url: str + ) -> None: + """get_iter follows has_next instead of stopping at the first page.""" + pages = [ + Response( + 200, + json={ + "success": True, + "data": {"mappings": [{"target_concept_id": 1}]}, + "meta": {"pagination": {"has_next": True, "total_items": 2}}, + }, + ), + Response( + 200, + json={ + "success": True, + "data": {"mappings": [{"target_concept_id": 2}]}, + "meta": {"pagination": {"has_next": False, "total_items": 2}}, + }, + ), + ] + route = respx.get(f"{base_url}/concepts/201826/mappings").mock( + side_effect=pages + ) + + result = list(sync_client.mappings.get_iter(201826, page_size=1)) + + assert [m["target_concept_id"] for m in result] == [1, 2] + assert len(route.calls) == 2 + assert "page=2" in str(route.calls[1].request.url) + + @respx.mock + def test_get_iter_stops_without_pagination_meta( + self, sync_client: OMOPHub, base_url: str + ) -> None: + """A response with no pagination meta terminates rather than looping.""" + route = respx.get(f"{base_url}/concepts/201826/mappings").mock( + return_value=Response( + 200, + json={ + "success": True, + "data": {"mappings": [{"target_concept_id": 1}]}, + }, + ) + ) + + result = list(sync_client.mappings.get_iter(201826)) + + assert len(result) == 1 + assert len(route.calls) == 1 + @respx.mock def test_map_concepts(self, sync_client: OMOPHub, base_url: str) -> None: """Test mapping concepts to a target vocabulary.""" @@ -200,6 +324,40 @@ async def test_async_get_mappings_with_filters( assert "target_vocabulary=ICD10CM" in url_str assert "include_invalid=true" in url_str + @pytest.mark.asyncio + @respx.mock + async def test_async_get_iter_walks_every_page( + self, async_client: omophub.AsyncOMOPHub, base_url: str + ) -> None: + """Async get_iter follows has_next across pages.""" + pages = [ + Response( + 200, + json={ + "success": True, + "data": {"mappings": [{"target_concept_id": 1}]}, + "meta": {"pagination": {"has_next": True, "total_items": 2}}, + }, + ), + Response( + 200, + json={ + "success": True, + "data": {"mappings": [{"target_concept_id": 2}]}, + "meta": {"pagination": {"has_next": False, "total_items": 2}}, + }, + ), + ] + route = respx.get(f"{base_url}/concepts/201826/mappings").mock( + side_effect=pages + ) + + result = [m async for m in async_client.mappings.get_iter(201826, page_size=1)] + + assert [m["target_concept_id"] for m in result] == [1, 2] + assert len(route.calls) == 2 + assert "page=2" in str(route.calls[1].request.url) + @pytest.mark.asyncio @respx.mock async def test_async_map_concepts(