util: Implement Paginator class as interface to access a list by pages
#11247
Conversation
chia/util/paginator.py
Outdated
| _source: Sequence[object] | ||
| _page_size: int | ||
|
|
||
| page_sizes: Tuple[int, ...] = (5, 10, 25, 50, 100) |
There was a problem hiding this comment.
I don't think this restriction improves the class.
| def page_count(self) -> int: | ||
| return max(1, ceil(len(self._source) / self._page_size)) | ||
|
|
||
| def get_page(self, page: int) -> Sequence[object]: |
There was a problem hiding this comment.
Do we really want 1-based paging instead of 0-based? I don't know what is normal for HTTP APIs.
There was a problem hiding this comment.
I don't know either. But i changed it to 0-based now because i was also not super convinced about the decision, see a948633
xdustinface
force-pushed
the
pr-util-paginator
branch
from
84405ad to
34a41c1
Compare
|
@altendky See last few commits, improved tests (kind of mixed in commits with the other two changes), addressed the comments and rebased to get the correct workflows. |
chia/util/paginator.py
Outdated
| return max(1, ceil(len(self._source) / self._page_size)) | ||
|
|
||
| def get_page(self, page: int) -> Sequence[object]: | ||
| if page < 0 or page > self.page_count() - 1: |
There was a problem hiding this comment.
| if page < 0 or page > self.page_count() - 1: | |
| if 0 <= page < self.page_count(): |
There was a problem hiding this comment.
You wanna raise for valid values?
There was a problem hiding this comment.
uh... right... i like 'em in order. (remember when you had an approval and gave me more chances to complain? :]) but really, whatever here.
| if page < 0 or page > self.page_count() - 1: | |
| if page < 0 or self.page_count() <= page: |
There was a problem hiding this comment.
I almost like it, but what about 04d9b60 😄 (swap the order)
tests/util/test_paginator.py
Outdated
| ], | ||
| ) | ||
| def test_get_page_valid(length: int, page: int, page_size: int, expected_data: List[int]) -> None: | ||
| assert Paginator(list(range(0, length)), page_size).get_page(page) == expected_data |
There was a problem hiding this comment.
:] maybe?
| assert Paginator(list(range(0, length)), page_size).get_page(page) == expected_data | |
| assert Paginator(range(length), page_size).get_page(page) == expected_data |
There was a problem hiding this comment.
expected_data is no range though :)
…ges (#11247) * util: Implement `Paginator` class as interface to access a list by pages * Be less restrictive about page sizes and refactor tests * Make the pages based of 0 instead of 1 and some more test refactoring * More tests * Adjust workflows after rebase * Introduce `Paginator.create` * `<=` instead of `- 1`
…ges (#11247) * util: Implement `Paginator` class as interface to access a list by pages * Be less restrictive about page sizes and refactor tests * Make the pages based of 0 instead of 1 and some more test refactoring * More tests * Adjust workflows after rebase * Introduce `Paginator.create` * `<=` instead of `- 1`
Preparation for a follow up PR to provide paginated plot RPC endpoints. Im fine waiting with the merge until that other PR is ready, just opening this to already get it reviewed.