feat: Add pagination buttons (prev/next) to anywidget mode for DataFrames #1841
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
8125614 to
0f871a9
Compare
87958ac to
3f87072
Compare
36f89e2 to
6f70a4a
Compare
e01a330 to
0586866
Compare
c4642e6 to
98eba30
Compare
|
|
||
| # Initialize data fetching attributes. | ||
| self._batches = dataframe.to_pandas_batches(page_size=self.page_size) | ||
| self._cached_data = pd.DataFrame(columns=self._dataframe.columns) |
There was a problem hiding this comment.
Mutating a DataFrame can be kinda slow (lots of potential for unnecessary memory copies). Perhaps we can use a list of DataFrame instead?
Or are you trying to support dynamic changing of the page size here? If so, perhaps leave a comment explaining that.
There was a problem hiding this comment.
I use a list of DataFrame instead.
| # Store the widget for _repr_mimebundle_ to use | ||
| self._anywidget_instance = display.TableWidget(self) | ||
| # Return a fallback HTML string | ||
| return "Interactive table widget (anywidget mode)" |
There was a problem hiding this comment.
Should we be calling _anywidget_instance._repr_html_ here? Or display on the widget instance? See jupyter-widgets/ipywidgets#3843 as well as my comment in the notebook about the warning.
| warnings.warn( | ||
| "Anywidget mode is not available, falling back to deferred mode." | ||
| ) | ||
| return formatter.repr_query_job(self._compute_dry_run()) | ||
|
|
||
| self._cached() |
There was a problem hiding this comment.
We never make it to this code or the blob_display code or the column width code that follows if they set anywidget as the mode. Are you sure that's what we want? At some point (maybe BigFrames 3.0 or maybe even earlier) we'll want to make anywidget the default, so it would be disappointing to lose these features.
Maybe we need some TODOs and follow-up issues filed to consolidate these code paths?
| self._all_data_loaded = True | ||
| # update row count if we loaded all data | ||
| if self.row_count == 0: | ||
| self.row_count = len(self._cached_data) |
There was a problem hiding this comment.
I'm a bit confused what the purpose of this is. Didn't we set row_count already in the constructor?
Also, 0 is not a great placeholder for missing value, since 0 is a valid number of rows in a query result. If we do delay populating row_count, let's use None as the placeholder for missing value.
| except Exception as e: | ||
| raise RuntimeError(f"Error during batch processing: {str(e)}") from e |
There was a problem hiding this comment.
I'm curious what the purpose of this is? If it's not needed, it seems like it might be better just to let the underlying exception raise.
| except Exception as e: | |
| raise RuntimeError(f"Error during batch processing: {str(e)}") from e |
| except Exception as e: | ||
| raise RuntimeError(f"Error during batch processing: {str(e)}") from e | ||
|
|
||
| def _get_batch_iterator(self) -> Iterator[pd.DataFrame]: |
There was a problem hiding this comment.
Seems like we should use an @property for this.
Adding interactive pagination buttons controls (Previous/Next buttons) to the anywidget mode for BigFrames DataFrames, enabling users to navigate through large datasets page by page in Jupyter notebooks.
b/391599608