feat(data frame): Restore input.<ID>_selected_rows(). Rename input.<ID>_data_view_indices to input.<ID>_data_view_rows (#1377)

Author: schloerke

CHANGELOG.md

@@ -7,7 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [UNRELEASED]
 
-### Breaking Changes
+### `input` key changes
+
+* Restored `@render.data_frame`'s (prematurely removed in v0.9.0) input value `input.<ID>_selected_rows()`. Please use `<ID>.input_cell_selection()["rows"]` and consider `input.<ID>_selected_rows()` deprecated. (#1345, #1377)
+
+* `@render.data_frame`'s input value `input.<ID>_data_view_indices` has been renamed to `input.<ID>_data_view_rows` for consistent naming. Please use `input.<ID>_data_view_rows` and consider `input.<ID>_data_view_indices` deprecated. (#1377)
 
 ### New features
 
@@ -21,6 +25,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 * Fixed an issue that prevented Shiny from serving the `font.css` file referenced in Shiny's Bootstrap CSS file. (#1342)
 
+* Removed temporary state where a data frame renderer would try to subset to selected rows that did not exist. (#1351, #1377)
+
 ### Other changes
 
 * `Session` is now an abstract base class, and `AppSession` is a concrete subclass of it. Also, `ExpressMockSession` has been renamed `ExpressStubSession` and is a concrete subclass of `Session`. (#1331)

js/data-frame/index.tsx

@@ -320,7 +320,6 @@ const ShinyDataGrid: FC<ShinyDataGridProps<unknown>> = ({
 
   useEffect(() => {
     if (!id) return;
-    const shinyId = `${id}_cell_selection`;
     let shinyValue: CellSelection | null = null;
     if (rowSelectionModes.is_none()) {
       shinyValue = null;
@@ -334,28 +333,28 @@ const ShinyDataGrid: FC<ShinyDataGridProps<unknown>> = ({
     } else {
       console.error("Unhandled row selection mode:", rowSelectionModes);
     }
-    Shiny.setInputValue!(shinyId, shinyValue);
+    Shiny.setInputValue!(`${id}_cell_selection`, shinyValue);
   }, [id, rowSelection, rowSelectionModes, table, table.getSortedRowModel]);
 
   useEffect(() => {
     if (!id) return;
-    const shinyId = `${id}_column_sort`;
-    Shiny.setInputValue!(shinyId, sorting);
+    Shiny.setInputValue!(`${id}_column_sort`, sorting);
   }, [id, sorting]);
   useEffect(() => {
     if (!id) return;
-    const shinyId = `${id}_column_filter`;
-    Shiny.setInputValue!(shinyId, columnFilters);
+    Shiny.setInputValue!(`${id}_column_filter`, columnFilters);
   }, [id, columnFilters]);
   useEffect(() => {
     if (!id) return;
-    const shinyId = `${id}_data_view_indices`;
 
     // Already prefiltered rows!
     const shinyValue: RowModel<unknown[]> = table.getSortedRowModel();
 
     const rowIndices = table.getSortedRowModel().rows.map((row) => row.index);
-    Shiny.setInputValue!(shinyId, rowIndices);
+    Shiny.setInputValue!(`${id}_data_view_rows`, rowIndices);
+
+    // Legacy value as of 2024-05-13
+    Shiny.setInputValue!(`${id}_data_view_indices`, rowIndices);
   }, [
     id,
     table,
@@ -364,6 +363,18 @@ const ShinyDataGrid: FC<ShinyDataGridProps<unknown>> = ({
     columnFilters,
   ]);
 
+  // Restored for legacy purposes. Only send selected rows to Shiny when row selection is performed.
+  useEffect(() => {
+    if (!id) return;
+    let shinyValue: number[] | null = null;
+    if (rowSelectionModes.row !== SelectionModes._rowEnum.NONE) {
+      const rowSelectionKeys = rowSelection.keys().toList();
+      const rowsById = table.getSortedRowModel().rowsById;
+      shinyValue = rowSelectionKeys.map((key) => rowsById[key].index).sort();
+    }
+    Shiny.setInputValue!(`${id}_selected_rows`, shinyValue);
+  }, [id, rowSelection, rowSelectionModes, table]);
+
   // ### End row selection ############################################################
 
   // ### Editable cells ###############################################################

shiny/render/_data_frame.py

@@ -297,7 +297,7 @@ def data_view(self, *, selected: bool = False) -> pd.DataFrame:
           selected cells.
     """
 
-    _input_data_view_indices: reactive.Calc_[list[int]]
+    _input_data_view_rows: reactive.Calc_[list[int]]
     """
     Reactive value of the data frame's view indices.
 
@@ -403,13 +403,13 @@ def self_input_cell_selection() -> CellSelection | None:
         # self._input_column_filter = self__input_column_filter
 
         @reactive.calc
-        def self__input_data_view_indices() -> list[int]:
-            data_view_indices = self._get_session().input[
+        def self__input_data_view_rows() -> list[int]:
+            data_view_rows = self._get_session().input[
                 f"{self.output_id}_data_view_indices"
             ]()
-            return data_view_indices
+            return data_view_rows
 
-        self._input_data_view_indices = self__input_data_view_indices
+        self._input_data_view_rows = self__input_data_view_rows
 
         # @reactive.calc
         # def self__data_selected() -> pd.DataFrame:
@@ -485,23 +485,25 @@ def _subset_data_view(selected: bool) -> pd.DataFrame:
                 data = self._data_patched().copy(deep=False)
 
                 # Turn into list for pandas compatibility
-                data_view_indices = list(self._input_data_view_indices())
+                data_view_rows = list(self._input_data_view_rows())
 
                 # Possibly subset the indices to selected rows
                 if selected:
                     cell_selection = self.input_cell_selection()
                     if cell_selection is not None and cell_selection["type"] == "row":
                         # Use a `set` for faster lookups
-                        selected_row_indices_set = set(cell_selection["rows"])
-
-                        # Subset the data view indices to only include the selected rows
-                        data_view_indices = [
-                            index
-                            for index in data_view_indices
-                            if index in selected_row_indices_set
+                        selected_row_set = set(cell_selection["rows"])
+                        nrow = data.shape[0]
+
+                        # Subset the data view indices to only include the selected rows that are in the data
+                        data_view_rows = [
+                            row
+                            for row in data_view_rows
+                            # Make sure the row is not larger than the number of rows
+                            if row in selected_row_set and row < nrow
                         ]
 
-                return data.iloc[data_view_indices]
+                return data.iloc[data_view_rows]
 
         # Helper reactives so that internal calculations can be cached for use in other calculations
         @reactive.calc

shiny/www/shared/py-shiny/data-frame/data-frame.js

@@ -0,0 +1,37 @@
+import pandas as pd
+
+from shiny import App, Inputs, reactive, render, req, ui
+
+app_ui = ui.page_fluid(
+    ui.output_data_frame("df1"),
+    ui.output_text_verbatim("selected_rows", placeholder=True),
+    ui.output_text_verbatim("cell_selection", placeholder=True),
+)
+
+
+def server(input: Inputs):
+    df = reactive.Value(pd.DataFrame([[1, 2], [3, 4], [5, 6]], columns=["A", "B"]))
+
+    @render.data_frame
+    def df1():
+        return render.DataGrid(df(), selection_mode="rows")
+
+    @render.text
+    def selected_rows():
+        return f"Input selected rows: {input.df1_selected_rows()}"
+
+    @render.text
+    def cell_selection():
+        cell_selection = df1.input_cell_selection()
+        if cell_selection is None:
+            req(cell_selection)
+            raise ValueError("Cell selection is None")
+        if cell_selection["type"] != "row":
+            raise ValueError(
+                f"Cell selection type is not 'row': {cell_selection['type']}"
+            )
+        rows = cell_selection["rows"]
+        return f"Cell selection rows: {rows}"
+
+
+app = App(app_ui, server)

shiny/www/shared/py-shiny/data-frame/data-frame.js.map

@@ -0,0 +1,22 @@
+from __future__ import annotations
+
+from conftest import ShinyAppProc
+from controls import OutputDataFrame, OutputTextVerbatim
+from playwright.sync_api import Page
+
+
+def test_row_selection(page: Page, local_app: ShinyAppProc) -> None:
+    page.goto(local_app.url)
+
+    df = OutputDataFrame(page, "df1")
+    selected_rows = OutputTextVerbatim(page, "selected_rows")
+    cell_selection = OutputTextVerbatim(page, "cell_selection")
+
+    df.expect_n_row(3)
+    selected_rows.expect_value("Input selected rows: ()")
+    cell_selection.expect_value("Cell selection rows: ()")
+
+    df.select_rows([0, 2])
+
+    selected_rows.expect_value("Input selected rows: (0, 2)")
+    cell_selection.expect_value("Cell selection rows: (0, 2)")

tests/playwright/shiny/bugs/1345-render-data-frame-input/app.py

@@ -0,0 +1,49 @@
+import pandas as pd
+
+from shiny import App, Inputs, reactive, render, ui
+
+app_ui = ui.page_fluid(
+    ui.markdown(
+        """
+    ## Description
+
+    When you add a row, click on it and click the clear button you get:
+
+
+    """
+    ),
+    ui.input_action_button("add_row", "Add row"),
+    ui.input_action_button("clear_table", "Clear table"),
+    ui.output_text_verbatim("number_of_selected_rows"),
+    ui.output_data_frame("df1"),
+)
+
+
+def server(input: Inputs):
+    df = reactive.Value(pd.DataFrame(columns=["A", "B"]))
+
+    @render.data_frame
+    def df1():
+        return render.DataGrid(df(), selection_mode="rows")
+
+    @reactive.effect
+    @reactive.event(input.add_row)
+    def _():
+        old_df = df()
+        new_df = pd.concat(  # pyright: ignore[reportUnknownMemberType]
+            [old_df, pd.DataFrame([[1, 2]], columns=["A", "B"])]
+        )
+        df.set(new_df)
+
+    @render.text
+    def number_of_selected_rows():
+        df_selected = df1.data_view(selected=True)
+        return f"Selected rows: {len(df_selected)}"
+
+    @reactive.effect
+    @reactive.event(input.clear_table)
+    def _():
+        df.set(pd.DataFrame(columns=["A", "B"]))
+
+
+app = App(app_ui, server)

tests/playwright/shiny/bugs/1345-render-data-frame-input/test_1345_input_selected_rows.py

@@ -0,0 +1,34 @@
+from __future__ import annotations
+
+from conftest import ShinyAppProc
+from controls import InputActionButton, OutputDataFrame, OutputTextVerbatim
+from playwright.sync_api import Page
+
+
+def test_row_selection(page: Page, local_app: ShinyAppProc) -> None:
+    page.goto(local_app.url)
+
+    df = OutputDataFrame(page, "df1")
+    add_row = InputActionButton(page, "add_row")
+    clear_table = InputActionButton(page, "clear_table")
+    selected_rows = OutputTextVerbatim(page, "number_of_selected_rows")
+
+    df.expect_n_row(0)
+    selected_rows.expect_value("Selected rows: 0")
+
+    add_row.click()
+
+    df.expect_n_row(1)
+    selected_rows.expect_value("Selected rows: 0")
+
+    df.cell_locator(0, 0).click()
+    df.select_rows([0])
+
+    df.expect_n_row(1)
+    selected_rows.expect_value("Selected rows: 1")
+
+    clear_table.click()
+    selected_rows.expect_value("Selected rows: 0")
+
+    bad_error_lines = [line for line in local_app.stderr._lines if "INFO:" not in line]
+    assert len(bad_error_lines) == 0, bad_error_lines

tests/playwright/shiny/bugs/1351-render-data-frame-selected/app.py

@@ -84,23 +84,22 @@ def summary_data():
 
     from shiny import reactive
 
-    @reactive.effect
-    def _():
-        print(
-            "Filters:",
-            summary_data._input_column_filter(),  # pyright: ignore[reportUnknownArgumentType,reportAttributeAccessIssue]
-        )
-
-    @reactive.effect
-    def _():
-        print(
-            "Sorting:",
-            summary_data._input_column_sort(),  # pyright: ignore[reportUnknownArgumentType,reportAttributeAccessIssue]
-        )
+    # @reactive.effect
+    # def _():
+    #     print(
+    #         "Filters:",
+    #         summary_data._input_column_filter(),  # pyright: ignore[reportUnknownArgumentType,reportAttributeAccessIssue]
+    #     )
+    # @reactive.effect
+    # def _():
+    #     print(
+    #         "Sorting:",
+    #         summary_data._input_column_sort(),  # pyright: ignore[reportUnknownArgumentType,reportAttributeAccessIssue]
+    #     )
 
     @reactive.effect
     def _():
-        print("indices:", summary_data._input_data_view_indices())
+        print("indices:", summary_data._input_data_view_rows())
 
     @reactive.effect
     def _():

tests/playwright/shiny/bugs/1351-render-data-frame-selected/test_1351_selected_row.py

@@ -20,7 +20,7 @@
     ),
     ui.output_data_frame("iris_df"),
     ui.h2("Data view indices"),
-    ui.output_text_verbatim("data_view_indices"),
+    ui.output_text_verbatim("data_view_rows"),
     ui.h2("Indices when view_selected=True"),
     ui.output_text_verbatim("data_view_selected_true"),
     ui.h2("Indices when view_selected=False"),
@@ -41,8 +41,8 @@ def iris_df():
         )
 
     @render.code  # pyright: ignore[reportArgumentType]
-    def data_view_indices():
-        return iris_df._input_data_view_indices()
+    def data_view_rows():
+        return iris_df._input_data_view_rows()
 
     @render.code  # pyright: ignore[reportArgumentType]
     def data_view_selected_false():  # pyright: ignore[reportUnknownParameterType]