[AL-6294] | Bug Fix | Support for setting a 'from cursor for dataset.data_rows() #1185
Conversation
vbrodsky
force-pushed
the
VB/respect-from-cursor_AL-6294
branch
from
6d5778e to
87567fe
Compare
kopreschko
left a comment
kopreschko
left a comment
There was a problem hiding this comment.
Just added a small observation
| super().__init__(*args, **kwargs) | ||
| self.cursor_path = cursor_path | ||
| self.next_cursor: Optional[Any] = None | ||
| self.next_cursor: Optional[Any] = kwargs.get('params', {}).get('from') |
There was a problem hiding this comment.
This change makes the code a bit obscure. To enhance readability and maintainability, what do you think about explicitly adding params to the arguments of the constructor?
There was a problem hiding this comment.
That is how I had it..
def __init__(self, cursor_path: List[str], params={}, *args, **kwargs):
but in the parent init mypy had issues with it... ok let me look at it again
There was a problem hiding this comment.
I had it like you suggested, but had issues similar to https://stackoverflow.com/questions/21764770/typeerror-got-multiple-values-for-argument with mypy.. so I would prob have to 're-add' it back into kwargs before calling the parent constructor. Let me see
labelbox/pagination.py
Outdated
| def __init__(self, cursor_path: List[str], *args, **kwargs): | ||
| super().__init__(*args, **kwargs) | ||
| def __init__(self, cursor_path: List[str], params={}, *args, **kwargs): | ||
| super().__init__(*args, **dict(kwargs, params=params)) |
There was a problem hiding this comment.
might be shorter to just use {**kwargs, **params}
but I think this is getting weirder, it's probably better to leave it as it was before.
vbrodsky
force-pushed
the
VB/respect-from-cursor_AL-6294
branch
from
42327e2 to
87567fe
Compare
No description provided.