Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
May 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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)
Expand Down
27 changes: 19 additions & 8 deletions js/data-frame/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand All @@ -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 ###############################################################
Expand Down
30 changes: 16 additions & 14 deletions shiny/render/_data_frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions shiny/www/shared/py-shiny/data-frame/data-frame.js

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions shiny/www/shared/py-shiny/data-frame/data-frame.js.map

Large diffs are not rendered by default.

37 changes: 37 additions & 0 deletions tests/playwright/shiny/bugs/1345-render-data-frame-input/app.py
Original file line number Diff line number Diff line change
@@ -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)
Original file line number Diff line number Diff line change
@@ -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)")
49 changes: 49 additions & 0 deletions tests/playwright/shiny/bugs/1351-render-data-frame-selected/app.py
Original file line number Diff line number Diff line change
@@ -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)
Original file line number Diff line number Diff line change
@@ -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
27 changes: 13 additions & 14 deletions tests/playwright/shiny/components/data_frame/edit/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 _():
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand All @@ -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]
Expand Down
Loading
Loading