Add pagination #21
Add pagination #21
Conversation
bunq/sdk/client.py
Outdated
| @@ -13,6 +13,11 @@ | |||
| from bunq.sdk import context | |||
| from bunq.sdk import exception | |||
|
|
|||
| try: | |||
There was a problem hiding this comment.
Why are we trying to import?
There was a problem hiding this comment.
It was needed to import different libs depending on the python version (one for 2^, the other for 3^). Removed the if-elses!
examples/payment_list_example.py
Outdated
| # Size of page of payments to list | ||
| _PAGE_SIZE = 3 | ||
|
|
||
| _USER_ITEM_ID = 1969 # Put your user ID here |
examples/payment_list_example.py
Outdated
| _PAGE_SIZE = 3 | ||
|
|
||
| _USER_ITEM_ID = 1969 # Put your user ID here | ||
| _MONETARY_ACCOUNT_ITEM_ID = 1988 # Put your monetary account ID here |
| :rtype: str | ||
| """ | ||
|
|
||
| if params: |
There was a problem hiding this comment.
Wouldn't this be always true?
There was a problem hiding this comment.
@OGKevin nope. Empty dictionaries evaluate to False! https://docs.python.org/2/library/stdtypes.html#truth-value-testing
|
@OGKevin Done, but let's not merge yet and look make sure the conf is OK first :) |
|
@dnl-blkv all tests passing, please include pagination tests as we discussed and then it should be ready for merging. |
|
@OGKevin this one is done; please take a look :) |
tests/config.py
Outdated
| """ | ||
| :rtype: str | ||
| :rtype: str[] |
There was a problem hiding this comment.
Hehe this is not how this is notated in python 😛
Please rewrite to list[str]
tests/config.py
Outdated
|
|
||
| if not permitted_ips_str: | ||
| return [] | ||
| else: |
There was a problem hiding this comment.
Isn't this Else redudndant ?
tests/test_pagination.py
Outdated
| pagination.older_id = None | ||
|
|
||
| def access_url_params_previous_page(): | ||
| _ = pagination.url_params_previous_page |
There was a problem hiding this comment.
Is there really a need to assign it to _ ? Can't you just call agination.url_params_previous_page without assigning it ?
There was a problem hiding this comment.
Without assignment it logically gets an IDE warning. This is a Google-accepted way of suppressing these IDE warnings. See: https://google.github.io/styleguide/pyguide.html#Python_Language_Rules
| :raise: NotImplementedError | ||
| """ | ||
|
|
||
| _ = pagination |
There was a problem hiding this comment.
Why does this need to be assigned to _ ?
There was a problem hiding this comment.
| pagination.future_id = None | ||
|
|
||
| def access_url_params_next_page(): | ||
| _ = pagination.url_params_next_page |
There was a problem hiding this comment.
tests/test_pagination_scenario.py
Outdated
| :rtype: None | ||
| """ | ||
|
|
||
| for i in range(self._payment_missing_count): |
There was a problem hiding this comment.
Please rewrite i with _ as of it is not being used inside the loop.
|
@OGKevin All addressed/answered; please take a look! |
tests/test_pagination.py
Outdated
| self.assertEqual(url_params_count_only_expected, | ||
| pagination.url_params_count_only) | ||
|
|
||
| def create_pagination_with_all_properties_set(self): |
There was a problem hiding this comment.
Shouldn't this be private ?
There was a problem hiding this comment.
@OGKevin that's true, fixed!
| 'Programming Language :: Python :: 3', | ||
| 'Programming Language :: Python :: 3.3', | ||
| 'Programming Language :: Python :: 3.4', | ||
| 'Programming Language :: Python :: 3.5', | ||
| 'Programming Language :: Python :: 3.6', | ||
| ], | ||
|
|
||
| # Require Python version equal or more than Python 3. | ||
| python_requires='>=3', |
There was a problem hiding this comment.
Python 2.7 handles dependencies poorly. So, it turned out the SDK was not working with 2.7 for last 2 weeks... However, no one ever reported the bug or complained. Moreover, some users were pushing for switch to Python 3 only.
No description provided.