diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 323ef8f07a..f839c3c0a4 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -46,4 +46,4 @@ repos: rev: v2.0.2 hooks: - id: biome-check - files: '\.js$' + files: '\.(js|css)$' diff --git a/MANIFEST.in b/MANIFEST.in index e0deb6deb2..c8555a39bf 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -17,7 +17,7 @@ # Generated by synthtool. DO NOT EDIT! include README.rst LICENSE recursive-include third_party/bigframes_vendored * -recursive-include bigframes *.json *.proto *.js py.typed +recursive-include bigframes *.json *.proto *.js *.css py.typed recursive-include tests * global-exclude *.py[co] global-exclude __pycache__ diff --git a/bigframes/display/anywidget.py b/bigframes/display/anywidget.py index 04d82c97fe..8bbb72c11a 100644 --- a/bigframes/display/anywidget.py +++ b/bigframes/display/anywidget.py @@ -62,20 +62,20 @@ def __init__(self, dataframe: bigframes.dataframe.DataFrame): super().__init__() self._dataframe = dataframe - # respect display options - self.page_size = bigframes.options.display.max_rows - - # Initialize data fetching attributes. - self._batches = dataframe.to_pandas_batches(page_size=self.page_size) - - # Use list of DataFrames to avoid memory copies from concatenation - self._cached_batches: List[pd.DataFrame] = [] - - # Unique identifier for HTML table element + # Initialize attributes that might be needed by observers FIRST self._table_id = str(uuid.uuid4()) self._all_data_loaded = False - # Renamed from _batch_iterator to _batch_iter to avoid naming conflict self._batch_iter: Optional[Iterator[pd.DataFrame]] = None + self._cached_batches: List[pd.DataFrame] = [] + + # respect display options for initial page size + initial_page_size = bigframes.options.display.max_rows + + # Initialize data fetching attributes. + self._batches = dataframe.to_pandas_batches(page_size=initial_page_size) + + # set traitlets properties that trigger observers + self.page_size = initial_page_size # len(dataframe) is expensive, since it will trigger a # SELECT COUNT(*) query. It is a must have however. @@ -91,18 +91,26 @@ def _esm(self): """Load JavaScript code from external file.""" return resources.read_text(bigframes.display, "table_widget.js") + @functools.cached_property + def _css(self): + """Load CSS code from external file.""" + return resources.read_text(bigframes.display, "table_widget.css") + page = traitlets.Int(0).tag(sync=True) page_size = traitlets.Int(25).tag(sync=True) row_count = traitlets.Int(0).tag(sync=True) table_html = traitlets.Unicode().tag(sync=True) @traitlets.validate("page") - def _validate_page(self, proposal: Dict[str, Any]): + def _validate_page(self, proposal: Dict[str, Any]) -> int: """Validate and clamp the page number to a valid range. Args: proposal: A dictionary from the traitlets library containing the proposed change. The new value is in proposal["value"]. + + Returns: + The validated and clamped page number as an integer. """ value = proposal["value"] @@ -115,11 +123,32 @@ def _validate_page(self, proposal: Dict[str, Any]): # Clamp the proposed value to the valid range [0, max_page]. return max(0, min(value, max_page)) + @traitlets.validate("page_size") + def _validate_page_size(self, proposal: Dict[str, Any]) -> int: + """Validate page size to ensure it's positive and reasonable. + + Args: + proposal: A dictionary from the traitlets library containing the + proposed change. The new value is in proposal["value"]. + + Returns: + The validated page size as an integer. + """ + value = proposal["value"] + + # Ensure page size is positive and within reasonable bounds + if value <= 0: + return self.page_size # Keep current value + + # Cap at reasonable maximum to prevent performance issues + max_page_size = 1000 + return min(value, max_page_size) + def _get_next_batch(self) -> bool: """ Gets the next batch of data from the generator and appends to cache. - Return: + Returns: True if a batch was successfully loaded, False otherwise. """ if self._all_data_loaded: @@ -148,6 +177,13 @@ def _cached_data(self) -> pd.DataFrame: return pd.DataFrame(columns=self._dataframe.columns) return pd.concat(self._cached_batches, ignore_index=True) + def _reset_batches_for_new_page_size(self): + """Reset the batch iterator when page size changes.""" + self._batches = self._dataframe.to_pandas_batches(page_size=self.page_size) + self._cached_batches = [] + self._batch_iter = None + self._all_data_loaded = False + def _set_table_html(self): """Sets the current html data based on the current page and page size.""" start = self.page * self.page_size @@ -174,6 +210,18 @@ def _set_table_html(self): ) @traitlets.observe("page") - def _page_changed(self, change): + def _page_changed(self, _change: Dict[str, Any]): """Handler for when the page number is changed from the frontend.""" self._set_table_html() + + @traitlets.observe("page_size") + def _page_size_changed(self, _change: Dict[str, Any]): + """Handler for when the page size is changed from the frontend.""" + # Reset the page to 0 when page size changes to avoid invalid page states + self.page = 0 + + # Reset batches to use new page size for future data fetching + self._reset_batches_for_new_page_size() + + # Update the table display + self._set_table_html() diff --git a/bigframes/display/table_widget.css b/bigframes/display/table_widget.css new file mode 100644 index 0000000000..790b6ae1bc --- /dev/null +++ b/bigframes/display/table_widget.css @@ -0,0 +1,76 @@ +/** + * Copyright 2025 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +.bigframes-widget .table-container { + max-height: 620px; + overflow: auto; +} + +.bigframes-widget .footer { + align-items: center; + display: flex; + font-size: 0.8rem; + padding-top: 8px; +} + +.bigframes-widget .footer > * { + flex: 1; +} + +.bigframes-widget .pagination { + align-items: center; + display: flex; + flex-direction: row; + gap: 4px; + justify-content: center; + padding: 4px; +} + +.bigframes-widget .page-size { + align-items: center; + display: flex; + flex-direction: row; + gap: 4px; + justify-content: end; +} + +.bigframes-widget table { + border-collapse: collapse; + text-align: left; + width: 100%; +} + +.bigframes-widget th { + background-color: var(--colab-primary-surface-color, var(--jp-layout-color0)); + /* Uncomment once we support sorting: cursor: pointer; */ + position: sticky; + top: 0; + z-index: 1; +} + +.bigframes-widget button { + cursor: pointer; + display: inline-block; + text-align: center; + text-decoration: none; + user-select: none; + vertical-align: middle; +} + +.bigframes-widget button:disabled { + opacity: 0.65; + pointer-events: none; +} diff --git a/bigframes/display/table_widget.js b/bigframes/display/table_widget.js index 71484af4d5..7fff72d1ba 100644 --- a/bigframes/display/table_widget.js +++ b/bigframes/display/table_widget.js @@ -15,81 +15,147 @@ */ const ModelProperty = { - TABLE_HTML: "table_html", - ROW_COUNT: "row_count", - PAGE_SIZE: "page_size", PAGE: "page", + PAGE_SIZE: "page_size", + ROW_COUNT: "row_count", + TABLE_HTML: "table_html", }; const Event = { + CHANGE: "change", CHANGE_TABLE_HTML: `change:${ModelProperty.TABLE_HTML}`, CLICK: "click", }; /** - * Renders a paginated table and its controls into a given element. + * Renders the interactive table widget. * @param {{ - * model: !Backbone.Model, - * el: !HTMLElement + * model: any, + * el: HTMLElement * }} options */ function render({ model, el }) { + // Main container with a unique class for CSS scoping const container = document.createElement("div"); - container.innerHTML = model.get(ModelProperty.TABLE_HTML); + container.classList.add("bigframes-widget"); + + // Structure + const tableContainer = document.createElement("div"); + const footer = document.createElement("div"); - const buttonContainer = document.createElement("div"); + // Footer: Total rows label + const rowCountLabel = document.createElement("div"); + + // Footer: Pagination controls + const paginationContainer = document.createElement("div"); const prevPage = document.createElement("button"); - const label = document.createElement("span"); + const paginationLabel = document.createElement("span"); const nextPage = document.createElement("button"); + // Footer: Page size controls + const pageSizeContainer = document.createElement("div"); + const pageSizeLabel = document.createElement("label"); + const pageSizeSelect = document.createElement("select"); + + // Add CSS classes + tableContainer.classList.add("table-container"); + footer.classList.add("footer"); + paginationContainer.classList.add("pagination"); + pageSizeContainer.classList.add("page-size"); + + // Configure pagination buttons prevPage.type = "button"; nextPage.type = "button"; prevPage.textContent = "Prev"; nextPage.textContent = "Next"; - /** Updates the button states and page label based on the model. */ + // Configure page size selector + pageSizeLabel.textContent = "Page Size"; + for (const size of [10, 25, 50, 100]) { + const option = document.createElement("option"); + option.value = size; + option.textContent = size; + if (size === model.get(ModelProperty.PAGE_SIZE)) { + option.selected = true; + } + pageSizeSelect.appendChild(option); + } + + /** Updates the footer states and page label based on the model. */ function updateButtonStates() { - const totalPages = Math.ceil( - model.get(ModelProperty.ROW_COUNT) / model.get(ModelProperty.PAGE_SIZE), - ); + const rowCount = model.get(ModelProperty.ROW_COUNT); + const pageSize = model.get(ModelProperty.PAGE_SIZE); const currentPage = model.get(ModelProperty.PAGE); + const totalPages = Math.ceil(rowCount / pageSize); - label.textContent = `Page ${currentPage + 1} of ${totalPages}`; + rowCountLabel.textContent = `${rowCount.toLocaleString()} total rows`; + paginationLabel.textContent = `Page ${currentPage + 1} of ${totalPages || 1}`; prevPage.disabled = currentPage === 0; nextPage.disabled = currentPage >= totalPages - 1; + pageSizeSelect.value = pageSize; } /** - * Updates the page in the model. - * @param {number} direction -1 for previous, 1 for next. + * Increments or decrements the page in the model. + * @param {number} direction - `1` for next, `-1` for previous. */ function handlePageChange(direction) { - const currentPage = model.get(ModelProperty.PAGE); - const newPage = Math.max(0, currentPage + direction); - if (newPage !== currentPage) { - model.set(ModelProperty.PAGE, newPage); + const current = model.get(ModelProperty.PAGE); + const next = current + direction; + model.set(ModelProperty.PAGE, next); + model.save_changes(); + } + + /** + * Handles changes to the page size from the dropdown. + * @param {number} size - The new page size. + */ + function handlePageSizeChange(size) { + const currentSize = model.get(ModelProperty.PAGE_SIZE); + if (size !== currentSize) { + model.set(ModelProperty.PAGE_SIZE, size); model.save_changes(); } } + /** Updates the HTML in the table container and refreshes button states. */ + function handleTableHTMLChange() { + // Note: Using innerHTML is safe here because the content is generated + // by a trusted backend (DataFrame.to_html). + tableContainer.innerHTML = model.get(ModelProperty.TABLE_HTML); + updateButtonStates(); + } + + // Add event listeners prevPage.addEventListener(Event.CLICK, () => handlePageChange(-1)); nextPage.addEventListener(Event.CLICK, () => handlePageChange(1)); - - model.on(Event.CHANGE_TABLE_HTML, () => { - // Note: Using innerHTML can be a security risk if the content is - // user-generated. Ensure 'table_html' is properly sanitized. - container.innerHTML = model.get(ModelProperty.TABLE_HTML); - updateButtonStates(); + pageSizeSelect.addEventListener(Event.CHANGE, (e) => { + const newSize = Number(e.target.value); + if (newSize) { + handlePageSizeChange(newSize); + } }); + model.on(Event.CHANGE_TABLE_HTML, handleTableHTMLChange); + + // Assemble the DOM + paginationContainer.appendChild(prevPage); + paginationContainer.appendChild(paginationLabel); + paginationContainer.appendChild(nextPage); - // Initial setup - updateButtonStates(); + pageSizeContainer.appendChild(pageSizeLabel); + pageSizeContainer.appendChild(pageSizeSelect); + + footer.appendChild(rowCountLabel); + footer.appendChild(paginationContainer); + footer.appendChild(pageSizeContainer); + + container.appendChild(tableContainer); + container.appendChild(footer); - buttonContainer.appendChild(prevPage); - buttonContainer.appendChild(label); - buttonContainer.appendChild(nextPage); el.appendChild(container); - el.appendChild(buttonContainer); + + // Initial render + handleTableHTMLChange(); } export default { render }; diff --git a/notebooks/dataframes/anywidget_mode.ipynb b/notebooks/dataframes/anywidget_mode.ipynb index 072e5c6504..f6380a9fd4 100644 --- a/notebooks/dataframes/anywidget_mode.ipynb +++ b/notebooks/dataframes/anywidget_mode.ipynb @@ -75,7 +75,7 @@ { "data": { "text/html": [ - "Query job 0b22b0f5-b952-4546-a969-41a89e343e9b is DONE. 0 Bytes processed. Open Job" + "Query job c5fcfd5e-1617-49c8-afa3-86ca21019de4 is DONE. 0 Bytes processed. Open Job" ], "text/plain": [ "" @@ -141,7 +141,7 @@ { "data": { "text/html": [ - "Query job 8e57da45-b6a7-44fb-8c4f-4b87058d94cb is DONE. 171.4 MB processed. Open Job" + "Query job ab900a53-5011-4e80-85d5-0ef2958598db is DONE. 171.4 MB processed. Open Job" ], "text/plain": [ "" @@ -153,7 +153,7 @@ { "data": { "application/vnd.jupyter.widget-view+json": { - "model_id": "4d00aaf284984cbc97483c651b9c5110", + "model_id": "bda63ba739dc4d5f83a5e18eb27b2686", "version_major": 2, "version_minor": 1 }, @@ -204,7 +204,7 @@ { "data": { "application/vnd.jupyter.widget-view+json": { - "model_id": "d4af4cf7d24d4f1c8e9c9b5f237df32b", + "model_id": "9bffeb73549e48419c3dd895edfe30e8", "version_major": 2, "version_minor": 1 }, @@ -290,7 +290,7 @@ { "data": { "application/vnd.jupyter.widget-view+json": { - "model_id": "0f04ad3c464145ee9735eba09f5107a9", + "model_id": "dfd4fa3a1c6f4b3eb1877cb0e9ba7e94", "version_major": 2, "version_minor": 1 }, diff --git a/owlbot.py b/owlbot.py index 5dc57a35b8..b9145d4367 100644 --- a/owlbot.py +++ b/owlbot.py @@ -107,6 +107,13 @@ "recursive-include bigframes *.json *.proto *.js py.typed", ) +# Include JavaScript and CSS files for display widgets +assert 1 == s.replace( # MANIFEST.in + ["MANIFEST.in"], + re.escape("recursive-include bigframes *.json *.proto *.js py.typed"), + "recursive-include bigframes *.json *.proto *.js *.css py.typed", +) + # Fixup the documentation. assert 1 == s.replace( # docs/conf.py ["docs/conf.py"], diff --git a/tests/system/small/test_anywidget.py b/tests/system/small/test_anywidget.py index b6dfb22934..8a91176dd9 100644 --- a/tests/system/small/test_anywidget.py +++ b/tests/system/small/test_anywidget.py @@ -167,35 +167,31 @@ def test_widget_display_should_show_first_page_on_load( _assert_html_matches_pandas_slice(html, expected_slice, paginated_pandas_df) -def test_widget_navigation_should_display_second_page( - table_widget, paginated_pandas_df: pd.DataFrame -): - """ - Given a widget, when the page is set to 1, then it should display - the second page of data. - """ - expected_slice = paginated_pandas_df.iloc[2:4] - - table_widget.page = 1 - html = table_widget.table_html - - assert table_widget.page == 1 - _assert_html_matches_pandas_slice(html, expected_slice, paginated_pandas_df) - - -def test_widget_navigation_should_display_last_page( - table_widget, paginated_pandas_df: pd.DataFrame +@pytest.mark.parametrize( + "page_number, start_row, end_row", + [ + (1, 2, 4), # Second page + (2, 4, 6), # Last page + ], + ids=["second_page", "last_page"], +) +def test_widget_navigation_should_display_correct_page( + table_widget, + paginated_pandas_df: pd.DataFrame, + page_number: int, + start_row: int, + end_row: int, ): """ - Given a widget, when the page is set to the last page (2), - then it should display the final page of data. + Given a widget, when the page is set, then it should display the correct + slice of data. """ - expected_slice = paginated_pandas_df.iloc[4:6] + expected_slice = paginated_pandas_df.iloc[start_row:end_row] - table_widget.page = 2 + table_widget.page = page_number html = table_widget.table_html - assert table_widget.page == 2 + assert table_widget.page == page_number _assert_html_matches_pandas_slice(html, expected_slice, paginated_pandas_df) @@ -338,6 +334,108 @@ def test_empty_widget_should_render_table_headers(empty_bf_df: bf.dataframe.Data assert "id" in html +def test_page_size_change_should_reset_current_page_to_zero(table_widget): + """ + Given a widget on a non-default page, When the page_size is changed, + Then the current page attribute should reset to 0. + """ + # Start on page 1 with an initial page size of 2. + table_widget.page = 1 + assert table_widget.page == 1 + + # Change the page size. + table_widget.page_size = 3 + + # The page number is reset to 0. + assert table_widget.page == 0 + + +def test_page_size_change_should_render_html_with_new_size( + table_widget, paginated_pandas_df: pd.DataFrame +): + """ + Given a widget, when the page_size is changed, + the rendered HTML should immediately reflect the new page size. + """ + # The widget is in its initial state with page_size=2. + # We expect the first 3 rows after the change. + expected_slice = paginated_pandas_df.iloc[0:3] + + # Change the page size. + table_widget.page_size = 3 + + # The HTML now contains the first 3 rows. + html = table_widget.table_html + _assert_html_matches_pandas_slice(html, expected_slice, paginated_pandas_df) + + +def test_navigation_after_page_size_change_should_use_new_size( + table_widget, paginated_pandas_df: pd.DataFrame +): + """ + Given a widget whose page size has been changed, When we navigate to the + next page, Then the pagination should use the new page size. + """ + # Change the page size to 3. + table_widget.page_size = 3 + # We expect the second page to contain rows 4-6 (indices 3-6). + expected_slice = paginated_pandas_df.iloc[3:6] + + # Navigate to the next page. + table_widget.page = 1 + + # The second page's HTML correctly reflects the new page size. + html = table_widget.table_html + _assert_html_matches_pandas_slice(html, expected_slice, paginated_pandas_df) + + +@pytest.mark.parametrize("invalid_size", [0, -5], ids=["zero", "negative"]) +def test_setting_invalid_page_size_should_be_ignored(table_widget, invalid_size: int): + """When the page size is set to an invalid number (<=0), the change should + be ignored.""" + # Set the initial page to 2. + initial_size = table_widget.page_size + assert initial_size == 2 + + # Attempt to set the page size to a invlaid size. + table_widget.page_size = invalid_size + + # The page size remains unchanged. + assert table_widget.page_size == initial_size + + +def test_setting_page_size_above_max_should_be_clamped(table_widget): + """ + Given a widget, when the page size is set to a value greater than the + allowed maximum, the page size should be clamped to the maximum value. + """ + # The maximum is hardcoded to 1000 in the implementation. + expected_clamped_size = 1000 + + # Attempt to set a very large page size. + table_widget.page_size = 9001 + + # The page size is clamped to the maximum. + assert table_widget.page_size == expected_clamped_size + + +def test_widget_creation_should_load_css_for_rendering(table_widget): + """ + Given a TableWidget is created, when its resources are accessed, + it should contain the CSS content required for styling. + """ + # The table_widget fixture creates the widget. + # No additional setup is needed. + + # Access the CSS content. + css_content = table_widget._css + + # The content is a non-empty string containing a known selector. + assert isinstance(css_content, str) + assert len(css_content) > 0 + assert ".bigframes-widget .footer" in css_content + + # TODO(shuowei): Add tests for custom index and multiindex # This may not be necessary for the SQL Cell use case but should be # considered for completeness.