From 5663083325bffef696973786d5ec3dca4e6fb0a1 Mon Sep 17 00:00:00 2001 From: fderuiter <127706008+fderuiter@users.noreply.github.com> Date: Fri, 27 Feb 2026 16:38:30 +0000 Subject: [PATCH] Refactor SDK to remove dead code and duplication - Remove redundant `get` and `async_get` methods in `JobsEndpoint`. - Delete unused `CreateEndpointMixin` and its module. - Extract site filtering logic from `SubjectsEndpoint` to `imednet.utils.filters.filter_by_attribute`. - Update `SubjectsEndpoint` to use the new utility. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> --- imednet/core/endpoint/mixins/__init__.py | 2 - imednet/core/endpoint/mixins/create.py | 87 ------------------------ imednet/endpoints/jobs.py | 38 ----------- imednet/endpoints/subjects.py | 4 +- imednet/utils/filters.py | 29 +++++++- 5 files changed, 30 insertions(+), 130 deletions(-) delete mode 100644 imednet/core/endpoint/mixins/create.py diff --git a/imednet/core/endpoint/mixins/__init__.py b/imednet/core/endpoint/mixins/__init__.py index 363de74d..776b1460 100644 --- a/imednet/core/endpoint/mixins/__init__.py +++ b/imednet/core/endpoint/mixins/__init__.py @@ -18,7 +18,6 @@ StrictListGetEndpoint, ) from .caching import CacheMixin -from .create import CreateEndpointMixin from .get import FilterGetEndpointMixin, PathGetEndpointMixin from .list import ListEndpointMixin from .params import ParamMixin @@ -27,7 +26,6 @@ __all__ = [ "AsyncPaginator", "CacheMixin", - "CreateEndpointMixin", "EdcListEndpoint", "EdcListGetEndpoint", "EdcListPathGetEndpoint", diff --git a/imednet/core/endpoint/mixins/create.py b/imednet/core/endpoint/mixins/create.py deleted file mode 100644 index 147459f0..00000000 --- a/imednet/core/endpoint/mixins/create.py +++ /dev/null @@ -1,87 +0,0 @@ -from __future__ import annotations - -from typing import Any, Callable, Dict, Generic, Optional, TypeVar, cast - -import httpx - -from imednet.core.protocols import AsyncRequestorProtocol, RequestorProtocol - -T_RESP = TypeVar("T_RESP") - - -class CreateEndpointMixin(Generic[T_RESP]): - """Mixin implementing creation logic.""" - - def _prepare_kwargs( - self, - json: Any = None, - data: Any = None, - headers: Optional[Dict[str, str]] = None, - ) -> Dict[str, Any]: - """ - Prepare keyword arguments for the request. - - Filters out None values to preserve default behavior. - """ - kwargs: Dict[str, Any] = {} - if json is not None: - kwargs["json"] = json - if data is not None: - kwargs["data"] = data - if headers is not None: - kwargs["headers"] = headers - return kwargs - - def _process_response( - self, - response: httpx.Response, - parse_func: Optional[Callable[[Any], T_RESP]] = None, - ) -> T_RESP: - """ - Process the API response and parse the result. - - Args: - response: The HTTP response object. - parse_func: Optional function to parse the JSON payload. - - Returns: - The parsed response object. - """ - payload = response.json() - if parse_func: - return parse_func(payload) - return cast(T_RESP, payload) - - def _create_sync( - self, - client: RequestorProtocol, - path: str, - *, - json: Any = None, - data: Any = None, - headers: Optional[Dict[str, str]] = None, - parse_func: Optional[Callable[[Any], T_RESP]] = None, - ) -> T_RESP: - """ - Execute a synchronous creation request (POST). - """ - kwargs = self._prepare_kwargs(json=json, data=data, headers=headers) - response = client.post(path, **kwargs) - return self._process_response(response, parse_func) - - async def _create_async( - self, - client: AsyncRequestorProtocol, - path: str, - *, - json: Any = None, - data: Any = None, - headers: Optional[Dict[str, str]] = None, - parse_func: Optional[Callable[[Any], T_RESP]] = None, - ) -> T_RESP: - """ - Execute an asynchronous creation request (POST). - """ - kwargs = self._prepare_kwargs(json=json, data=data, headers=headers) - response = await client.post(path, **kwargs) - return self._process_response(response, parse_func) diff --git a/imednet/endpoints/jobs.py b/imednet/endpoints/jobs.py index b3fc269d..41a23513 100644 --- a/imednet/endpoints/jobs.py +++ b/imednet/endpoints/jobs.py @@ -22,41 +22,3 @@ class JobsEndpoint(EdcListPathGetEndpoint[JobStatus]): def _raise_not_found(self, study_key: Optional[str], item_id: Any) -> None: raise ValueError(f"Job {item_id} not found in study {study_key}") - - def get(self, study_key: Optional[str], batch_id: str) -> JobStatus: - """ - Get a specific job by batch ID. - - This method performs a direct API request using the provided - ``batch_id``; it does not use caching or the ``refresh`` flag. - - Args: - study_key: Study identifier - batch_id: Batch ID of the job - - Returns: - JobStatus object with current state and timestamps - - Raises: - ValueError: If the job is not found - """ - return super().get(study_key, batch_id) - - async def async_get(self, study_key: Optional[str], batch_id: str) -> JobStatus: - """ - Asynchronously get a specific job by batch ID. - - This is the async variant of :meth:`get`. Like the sync version, - it issues a direct request by ``batch_id`` without any caching. - - Args: - study_key: Study identifier - batch_id: Batch ID of the job - - Returns: - JobStatus object with current state and timestamps - - Raises: - ValueError: If the job is not found - """ - return await super().async_get(study_key, batch_id) diff --git a/imednet/endpoints/subjects.py b/imednet/endpoints/subjects.py index 6d8ca587..64a17bd0 100644 --- a/imednet/endpoints/subjects.py +++ b/imednet/endpoints/subjects.py @@ -4,6 +4,7 @@ from imednet.core.endpoint.mixins import EdcListGetEndpoint from imednet.models.subjects import Subject +from imednet.utils.filters import filter_by_attribute class SubjectsEndpoint(EdcListGetEndpoint[Subject]): @@ -19,8 +20,7 @@ class SubjectsEndpoint(EdcListGetEndpoint[Subject]): def _filter_by_site(self, subjects: List[Subject], site_id: str | int) -> List[Subject]: # TUI Logic: Strict string comparison to handle int/str mismatch - target_site = str(site_id) - return [s for s in subjects if str(s.site_id) == target_site] + return filter_by_attribute(subjects, "site_id", site_id) def list_by_site(self, study_key: str, site_id: str | int) -> List[Subject]: """ diff --git a/imednet/utils/filters.py b/imednet/utils/filters.py index 0a98b0af..277b0176 100644 --- a/imednet/utils/filters.py +++ b/imednet/utils/filters.py @@ -7,11 +7,13 @@ import functools import re -from typing import Any, Dict, List, Tuple, Union +from typing import Any, Dict, List, Tuple, TypeVar, Union # Pre-compiled regex for performance to avoid re-compilation in loops _UNSAFE_CHARS_REGEX = re.compile(r"[^A-Za-z0-9_.-]") +T = TypeVar("T") + @functools.lru_cache(maxsize=128) def _snake_to_camel(text: str) -> str: @@ -67,3 +69,28 @@ def build_filter_string( else: parts.append(f"{camel_key}=={_format_filter_value(value)}") return and_connector.join(parts) + + +def filter_by_attribute(items: List[T], attr_name: str, target_value: Any) -> List[T]: + """ + Filter a list of objects by a specific attribute value using strict string comparison. + + This function handles the common case where API IDs might be returned as integers + or strings, ensuring consistent comparison by converting both to strings. + + Args: + items: List of objects to filter. + attr_name: The name of the attribute to check on each object. + target_value: The value to filter for. + + Returns: + A new list containing only the items where the attribute matches the target value. + """ + target_str = str(target_value) + filtered_items = [] + for item in items: + # Use getattr to safely access the attribute + attr_val = getattr(item, attr_name, None) + if attr_val is not None and str(attr_val) == target_str: + filtered_items.append(item) + return filtered_items