Skip to content

Commit f501e0e

Browse files
shuoweilshuoweil
committed
remove widget reuse
1 parent 59767a0 commit f501e0e

File tree

4 files changed

+68
-51
lines changed

4 files changed

+68
-51
lines changed

‎bigframes/dataframe.py‎

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -802,19 +802,10 @@ def _repr_html_(self) -> str:
802802

803803
from bigframes import display
804804

805-
# Check if widget instance already exists and reuse it
806-
widget = None
807-
if (
808-
hasattr(self, "_anywidget_instance")
809-
and self._anywidget_instance is not None
810-
):
811-
widget = self._anywidget_instance()
812-
813-
# If widget doesn't exist or was garbage collected, create a new one
814-
if widget is None:
815-
# Pass the processed dataframe (with blob URLs) to the widget
816-
widget = display.TableWidget(df)
817-
self._anywidget_instance = weakref.ref(widget)
805+
# Always create a new widget instance for each display call
806+
# This ensures that each cell gets its own widget and prevents
807+
# unintended sharing between cells
808+
widget = display.TableWidget(df.copy())
818809

819810
ipython_display(widget)
820811
return "" # Return empty string since we used display()

‎bigframes/display/anywidget.py‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -126,21 +126,23 @@ def _get_next_batch(self) -> bool:
126126
return False
127127

128128
try:
129-
iterator = self._batch_iterator()
129+
iterator = self._batch_iterator
130130
batch = next(iterator)
131131
self._cached_batches.append(batch)
132132
return True
133133
except StopIteration:
134134
self._all_data_loaded = True
135135
return False
136136

137+
@property
137138
def _batch_iterator(self) -> Iterator[pd.DataFrame]:
138139
"""Lazily initializes and returns the batch iterator."""
139140
if self._batch_iter is None:
140141
self._batch_iter = iter(self._batches)
141142
return self._batch_iter
142143

143-
def _get_cached_data(self) -> pd.DataFrame:
144+
@property
145+
def _cached_data(self) -> pd.DataFrame:
144146
"""Combine all cached batches into a single DataFrame."""
145147
if not self._cached_batches:
146148
return pd.DataFrame(columns=self._dataframe.columns)
@@ -152,10 +154,10 @@ def _set_table_html(self):
152154
end = start + self.page_size
153155

154156
# fetch more data if the requested page is outside our cache
155-
cached_data = self._get_cached_data()
157+
cached_data = self._cached_data
156158
while len(cached_data) < end and not self._all_data_loaded:
157159
if self._get_next_batch():
158-
cached_data = self._get_cached_data()
160+
cached_data = self._cached_data
159161
else:
160162
break
161163

‎notebooks/dataframes/anywidget_mode.ipynb‎

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@
7575
{
7676
"data": {
7777
"text/html": [
78-
"Query job ad1f38a1-8cfa-4df8-ad88-f5ee052e135e is DONE. 0 Bytes processed. <a target=\"_blank\" href=\"https://console.cloud.google.com/bigquery?project=bigframes-dev&j=bq:US:ad1f38a1-8cfa-4df8-ad88-f5ee052e135e&page=queryresults\">Open Job</a>"
78+
"Query job 0b22b0f5-b952-4546-a969-41a89e343e9b is DONE. 0 Bytes processed. <a target=\"_blank\" href=\"https://console.cloud.google.com/bigquery?project=bigframes-dev&j=bq:US:0b22b0f5-b952-4546-a969-41a89e343e9b&page=queryresults\">Open Job</a>"
7979
],
8080
"text/plain": [
8181
"<IPython.core.display.HTML object>"
@@ -141,7 +141,7 @@
141141
{
142142
"data": {
143143
"text/html": [
144-
"Query job 51ab6180-0ea3-4f9c-9cf0-02ae45ebb1be is DONE. 171.4 MB processed. <a target=\"_blank\" href=\"https://console.cloud.google.com/bigquery?project=bigframes-dev&j=bq:US:51ab6180-0ea3-4f9c-9cf0-02ae45ebb1be&page=queryresults\">Open Job</a>"
144+
"Query job 8e57da45-b6a7-44fb-8c4f-4b87058d94cb is DONE. 171.4 MB processed. <a target=\"_blank\" href=\"https://console.cloud.google.com/bigquery?project=bigframes-dev&j=bq:US:8e57da45-b6a7-44fb-8c4f-4b87058d94cb&page=queryresults\">Open Job</a>"
145145
],
146146
"text/plain": [
147147
"<IPython.core.display.HTML object>"
@@ -153,7 +153,7 @@
153153
{
154154
"data": {
155155
"application/vnd.jupyter.widget-view+json": {
156-
"model_id": "e12cd42c7db14ef0aa882f20553b6318",
156+
"model_id": "4d00aaf284984cbc97483c651b9c5110",
157157
"version_major": 2,
158158
"version_minor": 1
159159
},
@@ -204,7 +204,7 @@
204204
{
205205
"data": {
206206
"application/vnd.jupyter.widget-view+json": {
207-
"model_id": "b2fdf1fda7344d75aea47fc5de8d6729",
207+
"model_id": "d4af4cf7d24d4f1c8e9c9b5f237df32b",
208208
"version_major": 2,
209209
"version_minor": 1
210210
},
@@ -222,10 +222,10 @@
222222
"import math\n",
223223
" \n",
224224
"# Create widget programmatically \n",
225-
"widget = TableWidget(df) \n",
226-
"print(f\"Total pages: {math.ceil(widget.row_count / widget.page_size)}\") \n",
225+
"widget = TableWidget(df)\n",
226+
"print(f\"Total pages: {math.ceil(widget.row_count / widget.page_size)}\")\n",
227227
" \n",
228-
"# Display the widget \n",
228+
"# Display the widget\n",
229229
"widget"
230230
]
231231
},
@@ -254,15 +254,15 @@
254254
}
255255
],
256256
"source": [
257-
"# Simulate button clicks programmatically \n",
258-
"print(\"Current page:\", widget.page) \n",
259-
" \n",
260-
"# Go to next page \n",
261-
"widget.page = 1 \n",
262-
"print(\"After next:\", widget.page) \n",
263-
" \n",
264-
"# Go to previous page \n",
265-
"widget.page = 0 \n",
257+
"# Simulate button clicks programmatically\n",
258+
"print(\"Current page:\", widget.page)\n",
259+
"\n",
260+
"# Go to next page\n",
261+
"widget.page = 1\n",
262+
"print(\"After next:\", widget.page)\n",
263+
"\n",
264+
"# Go to previous page\n",
265+
"widget.page = 0\n",
266266
"print(\"After prev:\", widget.page)"
267267
]
268268
},
@@ -290,7 +290,7 @@
290290
{
291291
"data": {
292292
"application/vnd.jupyter.widget-view+json": {
293-
"model_id": "612cf38f9ab74908a1435b180b69ee95",
293+
"model_id": "0f04ad3c464145ee9735eba09f5107a9",
294294
"version_major": 2,
295295
"version_minor": 1
296296
},
@@ -304,10 +304,10 @@
304304
}
305305
],
306306
"source": [
307-
"# Test with very small dataset \n",
308-
"small_df = df.head(5) \n",
309-
"small_widget = TableWidget(small_df) \n",
310-
"print(f\"Small dataset pages: {math.ceil(small_widget.row_count / small_widget.page_size)}\") \n",
307+
"# Test with very small dataset\n",
308+
"small_df = df.head(5)\n",
309+
"small_widget = TableWidget(small_df)\n",
310+
"print(f\"Small dataset pages: {math.ceil(small_widget.row_count / small_widget.page_size)}\")\n",
311311
"small_widget"
312312
]
313313
}

‎tests/system/small/test_anywidget.py‎

Lines changed: 37 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -131,21 +131,26 @@ def _assert_html_matches_pandas_slice(
131131
assert row["page_indicator"] not in table_html
132132

133133

134-
def test_widget_initialization_should_set_default_state(
134+
def test_widget_initialization_should_calculate_total_row_count(
135135
paginated_bf_df: bf.dataframe.DataFrame,
136136
):
137-
"""
138-
A TableWidget should initialize with correct default values for the page,
139-
page size, and total row count.
140-
"""
141-
with bf.option_context("display.repr_mode", "anywidget", "display.max_rows", 2):
142-
from bigframes import display
137+
"""A TableWidget should correctly calculate the total row count on creation."""
138+
from bigframes import display
143139

140+
with bf.option_context("display.repr_mode", "anywidget", "display.max_rows", 2):
144141
widget = display.TableWidget(paginated_bf_df)
145142

146-
assert widget.page == 0
147-
assert widget.page_size == EXPECTED_PAGE_SIZE
148-
assert widget.row_count == EXPECTED_ROW_COUNT
143+
assert widget.row_count == EXPECTED_ROW_COUNT
144+
145+
146+
def test_widget_initialization_should_set_default_pagination(
147+
table_widget,
148+
):
149+
"""A TableWidget should initialize with page 0 and the correct page size."""
150+
# The `table_widget` fixture already creates the widget.
151+
# Assert its state.
152+
assert table_widget.page == 0
153+
assert table_widget.page_size == EXPECTED_PAGE_SIZE
149154

150155

151156
def test_widget_display_should_show_first_page_on_load(
@@ -310,9 +315,28 @@ def test_widget_page_size_should_be_immutable_after_creation(
310315
assert widget.page == 1 # Should remain on same page
311316

312317

313-
# TODO(b/428918844, shuowei): Add test for empty results once this bug is fixed
314-
# This test is blocked by b/428918844 which causes to_pandas_batches()
315-
# to return empty iterables for empty DataFrames.
318+
def test_empty_widget_should_have_zero_row_count(empty_bf_df: bf.dataframe.DataFrame):
319+
"""Given an empty DataFrame, the widget's row count should be 0."""
320+
with bf.option_context("display.repr_mode", "anywidget"):
321+
from bigframes.display import TableWidget
322+
323+
widget = TableWidget(empty_bf_df)
324+
325+
assert widget.row_count == 0
326+
327+
328+
def test_empty_widget_should_render_table_headers(empty_bf_df: bf.dataframe.DataFrame):
329+
"""Given an empty DataFrame, the widget should still render table headers."""
330+
with bf.option_context("display.repr_mode", "anywidget"):
331+
from bigframes.display import TableWidget
332+
333+
widget = TableWidget(empty_bf_df)
334+
335+
html = widget.table_html
336+
337+
assert "<table" in html
338+
assert "id" in html
339+
316340

317341
# TODO(shuowei): Add tests for custom index and multiindex
318342
# This may not be necessary for the SQL Cell use case but should be

0 commit comments

Comments
 (0)