Refactor paginator #425
Refactor paginator #425
Conversation
|
Merging to
|
InnocentBug
force-pushed
the
refactor-paginator
branch
2 times, most recently
from
bed4509
to
b0a432c
Compare
…ocess user supplied URLs too much.
InnocentBug
force-pushed
the
refactor-paginator
branch
from
b0a432c
to
80e7bb9
Compare
… connection error at right place.
|
Merge #429 first. |
| and all other results are on the next pages. | ||
| Using the Paginator object, the user can simply and easily flip through the results of the search. | ||
| The details, that results are listed as pages are hidden from the user. | ||
| The pages are automatically requested from the API as needed. |
There was a problem hiding this comment.
ok so this comment is the heart of the paginator functionality
| self._api = api | ||
| self._vocabulary = {} | ||
| self._db_schema = self._get_db_schema() | ||
|
|
There was a problem hiding this comment.
is this a constructor for a data schema class where it preloads all the schemas for the nodes and make that accessible throughout other parts of the code? did this constructor not really exist before ?
There was a problem hiding this comment.
it is not really a constructor, since it is just getting the JSON data as a dict from the API.
This call existed before, but was in a different location.
I moved it now into the constructor of the DataSchema class, since a DataSchema without loaded JSON schema is not useful.
| api, | ||
| url_path: str, | ||
| page_number: Union[int, None], | ||
| query: str, | ||
| ): |
There was a problem hiding this comment.
does this object init function take a page number ? just curious in a use case of this instantiation how does a user know what page number to pass in unless there is also a way to get a number of pages?
There was a problem hiding this comment.
It does take a page number, because some queries do not allow pages to be specified.
So in that case a user can specify None for the page.
Also the constructor of the class is mostly hidden from the user as only api.search instantiates them and returns it to the user.
We could give it a default 0 so it a user doesn't know the default is good.
There was a problem hiding this comment.
@InnocentBug could you give a quick example where the call to the paginator would look like to retrieve an already existing node? I left this comment on my sdk update as well because you left an original comment to be implementing api.search or paginator .
Description
This refactors the paginator.
Instead of having the user manually page through the pages of search results, this improved paginator is a python iterator.
So users, can just loop through the results with
for node in paginatoras you would expect from python.A new change is also that the paginator now directly returns nodes instead of JSON that can be converted to nodes.
Changes
Tests are adjusted to the new design.
Known Issues
Notes
Tagging @duboyal for visibility.
Checklist
CONTRIBUTORS.md) in the pull request source branch.