From 08236f6e2ea92a96048e2c0c4fbf38d894770d5f Mon Sep 17 00:00:00 2001 From: Leonardo Schwarz Date: Thu, 11 Dec 2025 14:49:27 +0100 Subject: [PATCH] Annotate References --- .basedpyright/baseline.bfabric.json | 306 ------------------ bfabric/docs/changelog.md | 1 + bfabric/src/bfabric/entities/core/entity.py | 6 +- .../src/bfabric/entities/core/references.py | 95 ++++-- pyproject.toml | 1 + tests/bfabric/entities/core/test_entity.py | 2 +- 6 files changed, 71 insertions(+), 340 deletions(-) diff --git a/.basedpyright/baseline.bfabric.json b/.basedpyright/baseline.bfabric.json index 51de78d1e..6c55f02da 100644 --- a/.basedpyright/baseline.bfabric.json +++ b/.basedpyright/baseline.bfabric.json @@ -1814,312 +1814,6 @@ } } ], - "./bfabric/src/bfabric/entities/core/references.py": [ - { - "code": "reportExplicitAny", - "range": { - "startColumn": 83, - "endColumn": 86, - "lineCount": 1 - } - }, - { - "code": "reportUnannotatedClassAttribute", - "range": { - "startColumn": 13, - "endColumn": 20, - "lineCount": 1 - } - }, - { - "code": "reportUnannotatedClassAttribute", - "range": { - "startColumn": 13, - "endColumn": 30, - "lineCount": 1 - } - }, - { - "code": "reportUnannotatedClassAttribute", - "range": { - "startColumn": 13, - "endColumn": 22, - "lineCount": 1 - } - }, - { - "code": "reportUnannotatedClassAttribute", - "range": { - "startColumn": 13, - "endColumn": 22, - "lineCount": 1 - } - }, - { - "code": "reportUnannotatedClassAttribute", - "range": { - "startColumn": 13, - "endColumn": 19, - "lineCount": 1 - } - }, - { - "code": "reportExplicitAny", - "range": { - "startColumn": 32, - "endColumn": 35, - "lineCount": 1 - } - }, - { - "code": "reportUnknownMemberType", - "range": { - "startColumn": 19, - "endColumn": 30, - "lineCount": 1 - } - }, - { - "code": "reportUnknownMemberType", - "range": { - "startColumn": 19, - "endColumn": 30, - "lineCount": 1 - } - }, - { - "code": "reportUnknownVariableType", - "range": { - "startColumn": 19, - "endColumn": 36, - "lineCount": 1 - } - }, - { - "code": "reportAny", - "range": { - "startColumn": 41, - "endColumn": 50, - "lineCount": 1 - } - }, - { - "code": "reportAny", - "range": { - "startColumn": 16, - "endColumn": 25, - "lineCount": 1 - } - }, - { - "code": "reportUnknownMemberType", - "range": { - "startColumn": 12, - "endColumn": 23, - "lineCount": 1 - } - }, - { - "code": "reportUnknownMemberType", - "range": { - "startColumn": 12, - "endColumn": 23, - "lineCount": 1 - } - }, - { - "code": "reportUnknownMemberType", - "range": { - "startColumn": 15, - "endColumn": 26, - "lineCount": 1 - } - }, - { - "code": "reportUnknownVariableType", - "range": { - "startColumn": 15, - "endColumn": 32, - "lineCount": 1 - } - }, - { - "code": "reportAny", - "range": { - "startColumn": 8, - "endColumn": 19, - "lineCount": 1 - } - }, - { - "code": "reportExplicitAny", - "range": { - "startColumn": 40, - "endColumn": 43, - "lineCount": 1 - } - }, - { - "code": "reportAny", - "range": { - "startColumn": 15, - "endColumn": 20, - "lineCount": 1 - } - }, - { - "code": "reportAny", - "range": { - "startColumn": 12, - "endColumn": 48, - "lineCount": 1 - } - }, - { - "code": "reportOptionalMemberAccess", - "range": { - "startColumn": 76, - "endColumn": 85, - "lineCount": 1 - } - }, - { - "code": "reportAny", - "range": { - "startColumn": 16, - "endColumn": 76, - "lineCount": 1 - } - }, - { - "code": "reportOptionalMemberAccess", - "range": { - "startColumn": 97, - "endColumn": 106, - "lineCount": 1 - } - }, - { - "code": "reportExplicitAny", - "range": { - "startColumn": 33, - "endColumn": 36, - "lineCount": 1 - } - }, - { - "code": "reportAny", - "range": { - "startColumn": 18, - "endColumn": 23, - "lineCount": 1 - } - }, - { - "code": "reportUnknownArgumentType", - "range": { - "startColumn": 20, - "endColumn": 30, - "lineCount": 1 - } - }, - { - "code": "reportAny", - "range": { - "startColumn": 24, - "endColumn": 29, - "lineCount": 1 - } - }, - { - "code": "reportExplicitAny", - "range": { - "startColumn": 31, - "endColumn": 34, - "lineCount": 1 - } - }, - { - "code": "reportUnknownArgumentType", - "range": { - "startColumn": 58, - "endColumn": 63, - "lineCount": 1 - } - }, - { - "code": "reportArgumentType", - "range": { - "startColumn": 58, - "endColumn": 69, - "lineCount": 1 - } - }, - { - "code": "reportArgumentType", - "range": { - "startColumn": 100, - "endColumn": 117, - "lineCount": 1 - } - }, - { - "code": "reportUnknownVariableType", - "range": { - "startColumn": 70, - "endColumn": 74, - "lineCount": 1 - } - }, - { - "code": "reportUnknownArgumentType", - "range": { - "startColumn": 63, - "endColumn": 67, - "lineCount": 1 - } - }, - { - "code": "reportUnknownVariableType", - "range": { - "startColumn": 91, - "endColumn": 95, - "lineCount": 1 - } - }, - { - "code": "reportArgumentType", - "range": { - "startColumn": 57, - "endColumn": 61, - "lineCount": 1 - } - }, - { - "code": "reportExplicitAny", - "range": { - "startColumn": 30, - "endColumn": 33, - "lineCount": 1 - } - }, - { - "code": "reportAny", - "range": { - "startColumn": 58, - "endColumn": 76, - "lineCount": 1 - } - }, - { - "code": "reportAny", - "range": { - "startColumn": 78, - "endColumn": 89, - "lineCount": 1 - } - } - ], "./bfabric/src/bfabric/entities/core/uri.py": [ { "code": "reportArgumentType", diff --git a/bfabric/docs/changelog.md b/bfabric/docs/changelog.md index ead690663..92caaf951 100644 --- a/bfabric/docs/changelog.md +++ b/bfabric/docs/changelog.md @@ -20,6 +20,7 @@ Minor breaking changes are still possible in `1.X.Y` but we try to announce them - Type hints have been narrowed in the public interfaces of the following classes: - `Bfabric` - `ResultContainer` + - `References` - `Entity` - `EntityReader` - `EngineSuds` diff --git a/bfabric/src/bfabric/entities/core/entity.py b/bfabric/src/bfabric/entities/core/entity.py index 242647201..6994581e1 100644 --- a/bfabric/src/bfabric/entities/core/entity.py +++ b/bfabric/src/bfabric/entities/core/entity.py @@ -1,3 +1,5 @@ +# pyright: reportImportCycles=false + from __future__ import annotations import warnings @@ -5,7 +7,6 @@ from typing import TYPE_CHECKING, Self, TypeGuard from bfabric.entities.core.mixins.find_mixin import FindMixin -from bfabric.entities.core.references import References from bfabric.entities.core.uri import EntityUri if TYPE_CHECKING: @@ -13,6 +14,7 @@ from typing import Any from bfabric import Bfabric + from bfabric.entities.core.references import References from bfabric.typing import ApiResponseDataType, ApiResponseObjectType @@ -86,6 +88,8 @@ def data_dict(self) -> ApiResponseObjectType: @cached_property def refs(self) -> References: """Returns the entity's references manager.""" + from bfabric.entities.core.references import References + return References(client=self._client, bfabric_instance=self.__bfabric_instance, data_ref=self.__data_dict) @property diff --git a/bfabric/src/bfabric/entities/core/references.py b/bfabric/src/bfabric/entities/core/references.py index fe6613d6f..a6a843ddb 100644 --- a/bfabric/src/bfabric/entities/core/references.py +++ b/bfabric/src/bfabric/entities/core/references.py @@ -1,6 +1,8 @@ +# pyright: reportImportCycles=false + from __future__ import annotations -from typing import Any, TYPE_CHECKING +from typing import TYPE_CHECKING, cast from loguru import logger from pydantic import BaseModel @@ -10,6 +12,8 @@ if TYPE_CHECKING: from bfabric import Bfabric + from bfabric.entities.core.entity import Entity + from bfabric.typing import ApiResponseDataType, ApiResponseObjectType class _ReferenceInformation(BaseModel): @@ -30,15 +34,16 @@ class References: This class receives a reference to the entity's data dictionary, updating it in-place when references are loaded. """ - def __init__(self, client: Bfabric, bfabric_instance: str, data_ref: dict[str, Any]) -> None: - self._client = client - self._bfabric_instance = bfabric_instance - self._data_ref = data_ref + def __init__(self, client: Bfabric, bfabric_instance: str, data_ref: ApiResponseObjectType) -> None: + self._client: Bfabric = client + self._bfabric_instance: str = bfabric_instance + self._data_ref: ApiResponseObjectType = data_ref # Retrieve information about all reference fields - self._ref_info = self.__extract_reference_info(data_ref=data_ref, bfabric_instance=self._bfabric_instance) - - self._cache = {} + self._ref_info: dict[str, _ReferenceInformation] = self.__extract_reference_info( + data_ref=data_ref, bfabric_instance=self._bfabric_instance + ) + self._cache: dict[str, Entity | list[Entity]] = {} @property def uris(self) -> dict[str, EntityUri | list[EntityUri]]: @@ -51,7 +56,7 @@ def is_loaded(self, name: str) -> bool: """ return self._ref_info[name].is_loaded - def get(self, name: str) -> Any | None: + def get(self, name: str) -> Entity | list[Entity] | None: if name in self._cache: return self._cache[name] @@ -61,18 +66,16 @@ def get(self, name: str) -> Any | None: if not ref_info.is_loaded: self.__load(ref_info=ref_info) - data_dicts = [self._data_ref[name]] if ref_info.is_singular else self._data_ref[name] + data_dicts = self.__get_ref_data_dicts(name, ref_info.is_singular) entities = [ instantiate_entity(data_dict=data_dict, client=self._client, bfabric_instance=self._bfabric_instance) for data_dict in data_dicts ] - if ref_info.is_singular: - self._cache[name] = entities[0] - else: - self._cache[name] = entities + + self._cache[name] = entities[0] if ref_info.is_singular else entities return self._cache[name] - def __getattr__(self, name: str) -> Any: + def __getattr__(self, name: str) -> Entity | list[Entity]: value = self.get(name) if value is None: raise AttributeError(f"'{type(self).__name__}' object has no attribute '{name}'") @@ -81,28 +84,47 @@ def __getattr__(self, name: str) -> Any: def __contains__(self, name: str) -> bool: return name in self._ref_info - def __load(self, ref_info: _ReferenceInformation) -> None: - from bfabric.entities.core.entity_reader import EntityReader - - reader = EntityReader.for_client(self._client) - entities = reader.read_uris(ref_info.uris) + def __get_ref_data_dicts(self, name: str, is_singular: bool) -> list[dict[str, ApiResponseDataType]]: + """Extract reference data as a list of dicts, using is_singular to determine the expected structure.""" + raw_data = self._data_ref[name] + if is_singular: + # is_singular=True means raw_data should be a dict + return [cast("dict[str, ApiResponseDataType]", raw_data)] + else: + # is_singular=False means raw_data should be a list of dicts + return cast("list[dict[str, ApiResponseDataType]]", raw_data) - # merge into the ref_data object + def __update_ref_data(self, ref_info: _ReferenceInformation, entities: dict[EntityUri, Entity | None]) -> None: + """Update reference data in-place with loaded entity data, handling None entities.""" if ref_info.is_singular: - self._data_ref[ref_info.name].update(entities[ref_info.uris[0]].data_dict) + ref_data = cast("dict[str, ApiResponseDataType]", self._data_ref[ref_info.name]) + entity = entities[ref_info.uris[0]] + if entity is None: + raise ValueError(f"Entity not found for URI: {ref_info.uris[0]}") + ref_data.update(entity.data_dict) else: + ref_data_list = cast("list[dict[str, ApiResponseDataType]]", self._data_ref[ref_info.name]) indices_map = {uri: idx for idx, uri in enumerate(ref_info.uris)} for entry_uri in ref_info.uris: - self._data_ref[ref_info.name][indices_map[entry_uri]].update(entities[entry_uri].data_dict) + entity = entities[entry_uri] + if entity is None: + raise ValueError(f"Entity not found for URI: {entry_uri}") + ref_data_list[indices_map[entry_uri]].update(entity.data_dict) + + def __load(self, ref_info: _ReferenceInformation) -> None: + from bfabric.entities.core.entity_reader import EntityReader - # mark as loaded + reader = EntityReader.for_client(self._client) + entities = reader.read_uris(ref_info.uris) + + self.__update_ref_data(ref_info, entities) ref_info.is_loaded = True @classmethod def __extract_reference_info( - cls, data_ref: dict[str, Any], bfabric_instance: str + cls, data_ref: ApiResponseObjectType, bfabric_instance: str ) -> dict[str, _ReferenceInformation]: - references = {} + references: dict[str, _ReferenceInformation] = {} for name, value in data_ref.items(): refs = cls.__extract_reference_info_item(name, value, bfabric_instance) if refs is not None: @@ -111,11 +133,13 @@ def __extract_reference_info( @classmethod def __extract_reference_info_item( - cls, name: str, value: Any, bfabric_instance: str + cls, name: str, value: ApiResponseDataType, bfabric_instance: str ) -> _ReferenceInformation | None: if isinstance(value, dict) and "classname" in value and "id" in value: info = cls.__extract_reference_info_item_dict(value, bfabric_instance) - return _ReferenceInformation(name=name, uris=[info["uri"]], is_singular=True, is_loaded=info["is_loaded"]) + return _ReferenceInformation.model_validate( + dict(name=name, uris=[info["uri"]], is_singular=True, is_loaded=info["is_loaded"]) + ) if isinstance(value, list) and all(isinstance(item, dict) for item in value): try: @@ -126,13 +150,20 @@ def __extract_reference_info_item( uris = [ref["uri"] for ref in refs] is_loaded = all(ref["is_loaded"] for ref in refs) - return _ReferenceInformation(name=name, uris=uris, is_singular=False, is_loaded=is_loaded) + return _ReferenceInformation.model_validate( + dict(name=name, uris=uris, is_singular=False, is_loaded=is_loaded) + ) @classmethod def __extract_reference_info_item_dict( - cls, value: dict[str, Any], bfabric_instance: str + cls, value: ApiResponseDataType, bfabric_instance: str ) -> dict[str, EntityUri | bool]: - uri = EntityUri.from_components(bfabric_instance, value["classname"], value["id"]) + # value is guaranteed to be a dict by the caller's isinstance check + value_dict = cast("dict[str, ApiResponseDataType]", value) + classname = cast("str", value_dict["classname"]) + entity_id = cast("int", value_dict["id"]) + + uri = EntityUri.from_components(bfabric_instance, classname, entity_id) # TODO double check if this handles the more complex references - is_loaded = len(value) > 2 + is_loaded = len(value_dict) > 2 return {"uri": uri, "is_loaded": is_loaded} diff --git a/pyproject.toml b/pyproject.toml index 092b0fb18..361790c70 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -43,6 +43,7 @@ reinstall-package = ["bfabric", "bfabric_scripts", "bfabric_app_runner", "bfabri [tool.basedpyright] exclude = [ "*/tests/*", + "**/.venv/*", # TODO this list should ideally not be extended or even consolidated at some point: "bfabric/src/bfabric/wrapper_creator/bfabric_submitter.py", "bfabric/src/bfabric/wrapper_creator/bfabric_wrapper_creator.py", diff --git a/tests/bfabric/entities/core/test_entity.py b/tests/bfabric/entities/core/test_entity.py index 96ca64ac8..1c7a5b385 100644 --- a/tests/bfabric/entities/core/test_entity.py +++ b/tests/bfabric/entities/core/test_entity.py @@ -44,7 +44,7 @@ def test_data_dict(mock_entity, mock_data_dict) -> None: def test_refs(mock_entity, mocker, mock_client, bfabric_instance) -> None: - mock_references = mocker.patch("bfabric.entities.core.entity.References") + mock_references = mocker.patch("bfabric.entities.core.references.References") assert mock_entity.refs == mock_references.return_value mock_references.assert_called_once_with( client=mock_client, bfabric_instance=bfabric_instance, data_ref=mock_entity.data_dict