From f3dd96deedd36825d4398af1203628854334542b Mon Sep 17 00:00:00 2001 From: Ludwig Schneider Date: Thu, 27 Jun 2024 08:43:09 -0500 Subject: [PATCH 1/6] adding child pagination --- src/cript/_client.py | 2 + src/cript/resources/__init__.py | 9 ++ src/cript/resources/child.py | 149 ++++++++++++++++++++++++++++++ tests/api_resources/test_cript.py | 55 ++++++++++- 4 files changed, 214 insertions(+), 1 deletion(-) create mode 100644 src/cript/resources/child.py diff --git a/src/cript/_client.py b/src/cript/_client.py index 3fc9bac..5faa06f 100644 --- a/src/cript/_client.py +++ b/src/cript/_client.py @@ -49,6 +49,7 @@ class Cript(SyncAPIClient): schema: resources.SchemaResource nodes: resources.NodesResource search: resources.SearchResource + _child: resources.child.ChildResource controlled_vocabularies: resources.ControlledVocabulariesResource with_raw_response: CriptWithRawResponse with_streaming_response: CriptWithStreamedResponse @@ -110,6 +111,7 @@ def __init__( self.schema = resources.SchemaResource(self) self.nodes = resources.NodesResource(self) self.search = resources.SearchResource(self) + self._child = resources.child.ChildResource(self) self.controlled_vocabularies = resources.ControlledVocabulariesResource(self) self.with_raw_response = CriptWithRawResponse(self) self.with_streaming_response = CriptWithStreamedResponse(self) diff --git a/src/cript/resources/__init__.py b/src/cript/resources/__init__.py index 45d9905..69cbd35 100644 --- a/src/cript/resources/__init__.py +++ b/src/cript/resources/__init__.py @@ -33,6 +33,12 @@ AsyncControlledVocabulariesResourceWithStreamingResponse, ) +from .child import ( + ChildResource, + ChildResourceWithRawResponse, + ChildResourceWithStreamingResponse, +) + __all__ = [ "SchemaResource", "AsyncSchemaResource", @@ -58,4 +64,7 @@ "AsyncControlledVocabulariesResourceWithRawResponse", "ControlledVocabulariesResourceWithStreamingResponse", "AsyncControlledVocabulariesResourceWithStreamingResponse", + "ChildResource", + "ChildResourceWithRawResponse", + "ChildResourceWithStreamingResponse", ] diff --git a/src/cript/resources/child.py b/src/cript/resources/child.py new file mode 100644 index 0000000..16831d5 --- /dev/null +++ b/src/cript/resources/child.py @@ -0,0 +1,149 @@ +import httpx +from .._types import NOT_GIVEN, Body, Query, Headers, NotGiven +from .._utils import ( + maybe_transform, + async_maybe_transform, +) +from .._compat import cached_property +from .._response import ( + to_raw_response_wrapper, + to_streamed_response_wrapper, + async_to_raw_response_wrapper, + async_to_streamed_response_wrapper, +) +from .._base_client import ( + make_request_options, +) +from ..types.shared.search import Search +from .._resource import SyncAPIResource + +class ChildPaginator: + # TODO consider writing operations + def __init__(self, parent, child, client=None): + if client is None: + client = parent.client + self._client = client + self._parent = parent + self._child = child + + self._current_child_list = [] + self._current_child_position = 0 + self._current_page = 0 + self._count = None + + def __iter__(self): + self._current_child_position = 0 + return self + + def __next__(self): + if self._current_child_position >= len(self._current_child_list): + self._fetch_next_page() + try: + next_node = self._current_child_list[self._current_child_position] + except IndexError: + raise StopIteration + + self._current_child_position += 1 + + return next_node + + def _fetch_next_page(self): + if self._finished_fetching: + raise StopIteration + + response = self._client._child.child(self._parent, self._child, self._current_page) + self._current_page += 1 + if self._count is not None and self._count != int(response.data.count): + raise RuntimeError("The number of elements for a child iteration changed during pagination. This may lead to inconsistencies. Please try again.") + self._count = int(response.data.count) + + self._current_child_list += response.data.result + + # Make it a random access iterator, since ppl expect it to behave list a list + def __getitem__(self, key): + key_index = int(key) + previous_pos = self._current_child_position + try: + if key_index < 0: + while not self._finished_fetching: + next(self) + + while len(self._current_child_list) <= key_index: + try: + next(self) + except StopIteration: + break + finally: + self._current_child_position = previous_pos + # We don't need explicit bounds checking, since the list access does that for us. + return self._current_child_list[key_index] + + def __len__(self): + previous_pos = self._current_child_position + try: + if self._count is None: + try: + next(iter(self)) + except StopIteration: + self._count = 0 + finally: + self._current_child_position = previous_pos + return self._count + + @property + def _finished_fetching(self): + if self._count is None: + return False + return len(self._current_child_list) == self._count + + +class ChildResource(SyncAPIResource): + @cached_property + def with_raw_response(self): + return ChildResourceWithRawResponse(self) + + @cached_property + def with_streaming_response(self): + return ChildResourceWithStreamingResponse(self) + + def child( + self, + parent, + child: str, + page: int, + *, + extra_headers: Headers | None = None, + extra_query: Query | None = None, + extra_body: Body | None = None, + timeout: float | httpx.Timeout | None | NotGiven = NOT_GIVEN, + ) -> Search: + """ + Obtain all children of parent node. + + Args: + parent: parent node + child: attribute name of the child node + """ + return self._get(f"/{parent.name_url}/{parent.uuid}/{child}", + options=make_request_options( + extra_headers=extra_headers, + extra_query=extra_query, + extra_body=extra_body, + query={"page": page}, + timeout=timeout, + ), + cast_to=Search, + ) + + +class ChildResourceWithRawResponse: + def __init__(self, child:ChildResource) -> None: + self._child = child + + self.node = to_raw_response_wrapper(child.node) + +class ChildResourceWithStreamingResponse: + def __init__(self, child:ChildResource) -> None: + self._child = child + + self.node = to_streamed_response_wrapper(child.node) diff --git a/tests/api_resources/test_cript.py b/tests/api_resources/test_cript.py index b1c2cde..9985c0b 100644 --- a/tests/api_resources/test_cript.py +++ b/tests/api_resources/test_cript.py @@ -7,6 +7,7 @@ import pytest +import cript from cript import * base_url = os.environ.get("TEST_API_BASE_URL", "http://127.0.0.1:4010") @@ -48,7 +49,7 @@ def test_create_project(self) -> None: notes="my notes", ) assert node.get("name") is not None - + def test_create_collection_exisiting_project(self) -> None: col1=Collection(name=generic_collection) proj = Project(uuid=CREATED_UUID, collection=[col1]) @@ -246,6 +247,58 @@ def test_unlink_all_children(self) -> None: proj1.delete(material=None) assert proj1.get("material") is None + def test_child_paginator(self)->None: + material_list = [] + num_mat = 15 + for i in range(num_mat): + mat1 = Material( + name=f"{generic_material1} #{i}", + bigsmiles="{[][<]CCO[>][]}", + ) + material_list += [mat1] + proj1 = Project(uuid=CREATED_UUID, material=material_list) + + paginator_iter = cript.resources.child.ChildPaginator(proj1, "material") + for i, child in enumerate(paginator_iter): + assert child.get("name").endswith(f"#{i}") + + paginator_len = cript.resources.child.ChildPaginator(proj1, "material") + # First time is doing it on empty + assert len(paginator_len) == num_mat + # Second time, it should have fetched + assert len(paginator_len) == num_mat + + paginator_rand = cript.resources.child.ChildPaginator(proj1, "material") + + # Accessing the second page right away + idx = 12 + child = paginator_rand[idx] + assert child.get("name").endswith(f"#{idx}") + # And again, shouldn't trigger a new fetch + idx = 13 + child = paginator_rand[idx] + assert child.get("name").endswith(f"#{idx}") + + with pytest.raises(IndexError): + paginator_rand[num_mat+4] + + paginator_neg = cript.resources.child.ChildPaginator(proj1, "material") + + # Accessing the second page right away + idx = -1 + child = paginator_neg[idx] + assert child.get("name").endswith(f"#{num_mat-1}") + + # Test list conversion + paginator_list = cript.resources.child.ChildPaginator(proj1, "material") + fetched_material_list = list(paginator_list) + for i, child in enumerate(fetched_material_list): + assert child.get("name").endswith(f"#{i}") + + # Test empty paginator + paginator_empty = cript.resources.child.ChildPaginator(proj1, "inventory") + assert len(paginator_empty) == 0 + def test_delete_node(self) -> None: proj1 = Project(uuid=CREATED_UUID) proj1.delete() From 8ab79766fcfef86359f0bc7dc3fc605c815829d6 Mon Sep 17 00:00:00 2001 From: Ludwig Schneider Date: Thu, 27 Jun 2024 09:02:58 -0500 Subject: [PATCH 2/6] implement child iteration --- src/cript/nodes/main.py | 9 ++++++++- tests/api_resources/test_cript.py | 8 ++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/cript/nodes/main.py b/src/cript/nodes/main.py index db95ecc..b83d06f 100644 --- a/src/cript/nodes/main.py +++ b/src/cript/nodes/main.py @@ -7,9 +7,11 @@ from jsonschema.exceptions import best_match from uuid import uuid4 +import cript from cript import Cript, NotFoundError, camel_case_to_snake_case, extract_node_from_result from .schema import cript_schema + logger = logging.getLogger(__name__) @@ -218,7 +220,12 @@ def __getattr__(self, key): try: return self.__getitem__(key) except KeyError: - raise AttributeError(key) + print("\nchild debug\n", self.children) + if key in self.children: + child_paginator = cript.resources.child.ChildPaginator(self, key) + return child_paginator + else: + raise AttributeError(key) def __setattr__(self, key, value): self.__setitem__(key, value) diff --git a/tests/api_resources/test_cript.py b/tests/api_resources/test_cript.py index 9985c0b..7e50983 100644 --- a/tests/api_resources/test_cript.py +++ b/tests/api_resources/test_cript.py @@ -67,6 +67,9 @@ def test_create_experiment_exisiting_collection(self) -> None: col1 = Collection(name=generic_collection, experiment=[exp1]) proj1 = Project(uuid=CREATED_UUID, collection=[col1]) assert exp1.get("name") == generic_experiment + assert proj1.collection[0].get("name") == col1.name + # TODO full node access + # assert proj1.collection[0].experiment[0].name == exp1.name def test_create_material(self) -> None: @@ -299,6 +302,11 @@ def test_child_paginator(self)->None: paginator_empty = cript.resources.child.ChildPaginator(proj1, "inventory") assert len(paginator_empty) == 0 + # Test empty paginator + paginator_non_exist = cript.resources.child.ChildPaginator(proj1, "non-existent attribute") + with pytest.raises(cript.NotFoundError): + len(paginator_non_exist) + def test_delete_node(self) -> None: proj1 = Project(uuid=CREATED_UUID) proj1.delete() From 28087c8d7590293b8243d71bb18969118f99b82d Mon Sep 17 00:00:00 2001 From: Ludwig Schneider Date: Thu, 27 Jun 2024 09:06:24 -0500 Subject: [PATCH 3/6] use attribute access in child pagination test at least once. --- src/cript/nodes/main.py | 2 +- tests/api_resources/test_cript.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cript/nodes/main.py b/src/cript/nodes/main.py index b83d06f..14bfa9a 100644 --- a/src/cript/nodes/main.py +++ b/src/cript/nodes/main.py @@ -220,7 +220,7 @@ def __getattr__(self, key): try: return self.__getitem__(key) except KeyError: - print("\nchild debug\n", self.children) + # TODO consider a caching of these paginators if key in self.children: child_paginator = cript.resources.child.ChildPaginator(self, key) return child_paginator diff --git a/tests/api_resources/test_cript.py b/tests/api_resources/test_cript.py index 7e50983..8ef50f2 100644 --- a/tests/api_resources/test_cript.py +++ b/tests/api_resources/test_cript.py @@ -261,7 +261,7 @@ def test_child_paginator(self)->None: material_list += [mat1] proj1 = Project(uuid=CREATED_UUID, material=material_list) - paginator_iter = cript.resources.child.ChildPaginator(proj1, "material") + paginator_iter = proj1.material for i, child in enumerate(paginator_iter): assert child.get("name").endswith(f"#{i}") From 101ee3b82aa45394277c68a071cda16b63460322 Mon Sep 17 00:00:00 2001 From: Ludwig Schneider Date: Mon, 8 Jul 2024 15:14:52 -0500 Subject: [PATCH 4/6] using existing resource --- src/cript/_client.py | 2 -- src/cript/nodes/main.py | 2 +- src/cript/resources/child.py | 5 ++++- src/cript/resources/nodes.py | 13 ++++++++++++- 4 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/cript/_client.py b/src/cript/_client.py index 5faa06f..3fc9bac 100644 --- a/src/cript/_client.py +++ b/src/cript/_client.py @@ -49,7 +49,6 @@ class Cript(SyncAPIClient): schema: resources.SchemaResource nodes: resources.NodesResource search: resources.SearchResource - _child: resources.child.ChildResource controlled_vocabularies: resources.ControlledVocabulariesResource with_raw_response: CriptWithRawResponse with_streaming_response: CriptWithStreamedResponse @@ -111,7 +110,6 @@ def __init__( self.schema = resources.SchemaResource(self) self.nodes = resources.NodesResource(self) self.search = resources.SearchResource(self) - self._child = resources.child.ChildResource(self) self.controlled_vocabularies = resources.ControlledVocabulariesResource(self) self.with_raw_response = CriptWithRawResponse(self) self.with_streaming_response = CriptWithStreamedResponse(self) diff --git a/src/cript/nodes/main.py b/src/cript/nodes/main.py index 14bfa9a..6c48b86 100644 --- a/src/cript/nodes/main.py +++ b/src/cript/nodes/main.py @@ -358,7 +358,7 @@ def retrieve_child(self, parent, child): try: if not self._primary_key: result = self.__dict__["client"].nodes.retrieve_children( - node=parent.name_url, uuid=parent.uuid, child_node=child.name_url + node=parent.name_url, uuid=parent.uuid, child_node=child.name_url, ) else: result = self.__dict__["client"].search.exact.child_node( diff --git a/src/cript/resources/child.py b/src/cript/resources/child.py index 16831d5..a42b696 100644 --- a/src/cript/resources/child.py +++ b/src/cript/resources/child.py @@ -28,6 +28,7 @@ def __init__(self, parent, child, client=None): self._current_child_list = [] self._current_child_position = 0 + # TODO change to after self._current_page = 0 self._count = None @@ -51,8 +52,10 @@ def _fetch_next_page(self): if self._finished_fetching: raise StopIteration - response = self._client._child.child(self._parent, self._child, self._current_page) + # TODO change to after + response = self._client.nodes.retrieve_children(uuid=self._parent.uuid, node=self._parent.name_url, child_node=self._child, page=self._current_page) self._current_page += 1 + if self._count is not None and self._count != int(response.data.count): raise RuntimeError("The number of elements for a child iteration changed during pagination. This may lead to inconsistencies. Please try again.") self._count = int(response.data.count) diff --git a/src/cript/resources/nodes.py b/src/cript/resources/nodes.py index 3ef9e46..0cbbccf 100644 --- a/src/cript/resources/nodes.py +++ b/src/cript/resources/nodes.py @@ -151,6 +151,8 @@ def retrieve_children( *, node: str, child_node: str, + # TODO change to after + page: int | None = None, # Use the following arguments if you need to pass additional parameters to the API that aren't available via kwargs. # The extra values given here take precedence over values defined on the client or passed to this method. extra_headers: Headers | None = None, @@ -176,10 +178,19 @@ def retrieve_children( raise ValueError(f"Expected a non-empty value for `uuid` but received {uuid!r}") if not child_node: raise ValueError(f"Expected a non-empty value for `child_node` but received {child_node!r}") + # TODO change to after + if page is not None: + query = {"page": page} + else: + query = {} # Does it make sense to allow non-paginated retrieval? The current Code uses it. return self._get( f"/{node}/{uuid}/{child_node}", options=make_request_options( - extra_headers=extra_headers, extra_query=extra_query, extra_body=extra_body, timeout=timeout + extra_headers=extra_headers, + extra_query=extra_query, + extra_body=extra_body, + timeout=timeout, + query=query, ), cast_to=Search, ) From ae4b50d04d39c5e2dd1cd796c8a26ccda29e8b02 Mon Sep 17 00:00:00 2001 From: Ludwig Schneider Date: Mon, 8 Jul 2024 15:19:58 -0500 Subject: [PATCH 5/6] remove duplication --- src/cript/resources/__init__.py | 8 ++--- src/cript/resources/child.py | 52 --------------------------------- 2 files changed, 2 insertions(+), 58 deletions(-) diff --git a/src/cript/resources/__init__.py b/src/cript/resources/__init__.py index 69cbd35..61e3032 100644 --- a/src/cript/resources/__init__.py +++ b/src/cript/resources/__init__.py @@ -34,9 +34,7 @@ ) from .child import ( - ChildResource, - ChildResourceWithRawResponse, - ChildResourceWithStreamingResponse, + ChildPaginator, ) __all__ = [ @@ -64,7 +62,5 @@ "AsyncControlledVocabulariesResourceWithRawResponse", "ControlledVocabulariesResourceWithStreamingResponse", "AsyncControlledVocabulariesResourceWithStreamingResponse", - "ChildResource", - "ChildResourceWithRawResponse", - "ChildResourceWithStreamingResponse", + "ChildPaginator", ] diff --git a/src/cript/resources/child.py b/src/cript/resources/child.py index a42b696..147aec1 100644 --- a/src/cript/resources/child.py +++ b/src/cript/resources/child.py @@ -98,55 +98,3 @@ def _finished_fetching(self): if self._count is None: return False return len(self._current_child_list) == self._count - - -class ChildResource(SyncAPIResource): - @cached_property - def with_raw_response(self): - return ChildResourceWithRawResponse(self) - - @cached_property - def with_streaming_response(self): - return ChildResourceWithStreamingResponse(self) - - def child( - self, - parent, - child: str, - page: int, - *, - extra_headers: Headers | None = None, - extra_query: Query | None = None, - extra_body: Body | None = None, - timeout: float | httpx.Timeout | None | NotGiven = NOT_GIVEN, - ) -> Search: - """ - Obtain all children of parent node. - - Args: - parent: parent node - child: attribute name of the child node - """ - return self._get(f"/{parent.name_url}/{parent.uuid}/{child}", - options=make_request_options( - extra_headers=extra_headers, - extra_query=extra_query, - extra_body=extra_body, - query={"page": page}, - timeout=timeout, - ), - cast_to=Search, - ) - - -class ChildResourceWithRawResponse: - def __init__(self, child:ChildResource) -> None: - self._child = child - - self.node = to_raw_response_wrapper(child.node) - -class ChildResourceWithStreamingResponse: - def __init__(self, child:ChildResource) -> None: - self._child = child - - self.node = to_streamed_response_wrapper(child.node) From 319197853d87787870c6bdb87e59a2a9026c736d Mon Sep 17 00:00:00 2001 From: Ludwig Schneider Date: Mon, 8 Jul 2024 15:25:32 -0500 Subject: [PATCH 6/6] remove duplicate --- src/cript/nodes/main.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/cript/nodes/main.py b/src/cript/nodes/main.py index 6c48b86..0eecdca 100644 --- a/src/cript/nodes/main.py +++ b/src/cript/nodes/main.py @@ -223,7 +223,8 @@ def __getattr__(self, key): # TODO consider a caching of these paginators if key in self.children: child_paginator = cript.resources.child.ChildPaginator(self, key) - return child_paginator + self.__dict__[key] = child_paginator + return self.__dict__[key] else: raise AttributeError(key)