Skip to content

Conversation

@bennettscience
Copy link
Contributor

Original PR (#539) from garth74:

This pull request makes two changes:

Adds type hinting to PaginatedList so that elements used in for-loops, list comprehensions, etc. are typed (more info).
Adds negative indexing to PaginatedList.

Related
Supports efforts of #435 and fixes #305 without preventing negative indexing all together (i.e., #306).

Updated to include upstream changes in develop since the original was opened in 2022.

garth74 and others added 5 commits July 25, 2022 10:05
This brings the PR up to speed with the current `develop` branch on
upstream. All tests passing, accounting for updates to `PaginatedList`
and `Requester` since the original PR was introduced.
@bennettscience
Copy link
Contributor Author

I think there are two things worth discussing:

  1. The typing improvements will help. I think PaginatedList is one of the harder bits of the library to get right if you've never used it before. It might help users in more complex cases avoid gotchas.
  2. My concern is about negative indexing large lists. It still has to get all of the results before you can access the results. Should we (I?) put in some more effort to address the other related issues (see PaginatedList redux #114, #114 Implement __len__() for paginated_list #237, and Add coverage for retrieving an activity stream #175.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Negative index on PaginatedList causes unexpected results

2 participants