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): Add .update_data(data, *, reset) and .update_cell_value(value, *, row, col) #1449

Open
maxmoro opened this issue Jun 4, 2024 · 7 comments
Labels
data frame Related to @render.data_frame enhancement New feature or request
Milestone

Comments

@maxmoro
Copy link

maxmoro commented Jun 4, 2024

In Shiny-R I often use the dataTableProxy() to manipulate the data shown in the DT, so the view is changed without reloading/regenerating the entire table.
Is it possible to do it with the data_frame in Python?

@maxmoro
Copy link
Author

maxmoro commented Jul 2, 2024

I'm trying to edit the data in the data_frame, without re-render the data_frame output, so I can keep the filter/sort the user performed.

Here is a test code, where when pressing the 'click' button, the second column of the second line should become 'xxx'. But it is not working

I'm using the .set_patches_fn inside the click event. I know it is not "by-the-book", I'm just wondering if it is possible in some way.

shiny Live link

and the code

from palmerpenguins import load_penguins
from shiny import App, render, ui, reactive, Outputs

penguins = load_penguins()

app_ui = ui.page_fluid(
    ui.h2("Palmer Penguins"),
    ui.input_action_button('click','click'),
    ui.output_ui("rows"),
    ui.output_data_frame("penguins_df"),
)

def server(input, output, session):
    

    @render.data_frame
    def penguins_df():
        return render.DataTable(penguins, editable=True,)  

    @reactive.Effect
    @reactive.event(input.click)
    def click():
        print('click')
        def edtx() -> list[render.CellPatch]():
            out = ({'row_index': 1, 'column_index': 1, 'value': 'xxx'},)
            print(out)
            return(out)
            
        penguins_df.set_patches_fn(edtx())
        
    #just testing if it works when editing another cell
    @penguins_df.set_patches_fn
    def edt(*, patches: list[render.CellPatch]) -> list[render.CellPatch]:
        out = ({'row_index': 1, 'column_index': 1, 'value': 'e'},)
        return(out)


app = App(app_ui, server)

@schloerke
Copy link
Collaborator

I've updated your app to use the new patches handler after being clicked.

There were two subtle changes:

  1. Do not call your function when setting it. Ex: penguins_df.set_patches_fn(edtx)
  2. Add *, patches parameters to edtx

Final app (shinylive):

from palmerpenguins import load_penguins

from shiny import App, reactive, render, ui

penguins = load_penguins()

app_ui = ui.page_fluid(
    ui.h2("Palmer Penguins"),
    ui.input_action_button("click", "click"),
    ui.output_ui("rows"),
    ui.output_data_frame("penguins_df"),
)


def server(input, output, session):

    @render.data_frame
    def penguins_df():
        return render.DataTable(
            penguins,
            editable=True,
        )

    @reactive.Effect
    @reactive.event(input.click)
    def click():
        print("click")

        def edtx(*, patches) -> list[render.CellPatch]:
            print("new!")
            out = [
                render.CellPatch(({"row_index": 1, "column_index": 1, "value": "xxx"}))
            ]
            print(out)
            return out

        penguins_df.set_patches_fn(edtx)

    # just testing if it works when editing another cell
    @penguins_df.set_patches_fn
    def edt(*, patches: list[render.CellPatch]) -> list[render.CellPatch]:
        print("original")
        out = [render.CellPatch(({"row_index": 1, "column_index": 1, "value": "e"}))]
        return out


app = App(app_ui, server)

I saw that the original cell location never escaped a saving state. This is being addressed in #1529 .

@schloerke schloerke added this to the v1.2.0 milestone Jul 15, 2024
@schloerke schloerke added enhancement New feature or request data frame Related to @render.data_frame and removed needs-triage labels Jul 15, 2024
@schloerke
Copy link
Collaborator

In Shiny-R I often use the dataTableProxy() to manipulate the data shown in the DT, so the view is changed without reloading/regenerating the entire table.
Is it possible to do it with the data_frame in Python?

It is definitely a possible feature! I have a sketch of what it could look like here:

# TODO-barret-render.data_frame; Add `update_cell_value()` method
# def _update_cell_value(
# self, value: CellValue, *, row_index: int, column_index: int
# ) -> CellPatch:
# """
# Update the value of a cell in the data frame.
#
# Parameters
# ----------
# value
# The new value to set the cell to.
# row_index
# The row index of the cell to update.
# column_index
# The column index of the cell to update.
# """
# cell_patch_processed = self._set_cell_patch_map_value(
# value, row_index=row_index, column_index=column_index
# )
# # TODO-barret-render.data_frame; Send message to client to update cell value
# return cell_patch_processed

It is currently not implemented as line 738 hints that we need a "send message to the browser" action that is not implemented in the typescript code. It would be similar to how we can update the sort from the server:

  • TS hook:
    useEffect(() => {
    const handleColumnSort = (
    event: CustomEvent<{ sort: { col: number; desc: boolean }[] }>
    ) => {
    const shinySorting = event.detail.sort;
    const columnSorting: SortingState = [];
    shinySorting.map((sort) => {
    columnSorting.push({
    id: columns[sort.col]!,
    desc: sort.desc,
    });
    });
    setSorting(columnSorting);
    };
    if (!id) return;
    const element = document.getElementById(id);
    if (!element) return;
    element.addEventListener(
    "updateColumnSort",
    handleColumnSort as EventListener
    );
    return () => {
    element.removeEventListener(
    "updateColumnSort",
    handleColumnSort as EventListener
    );
    };
    }, [columns, id, setSorting]);
  • Python code:
    await self._send_message_to_browser(
    "updateColumnSort",
    {"sort": vals},
    )

Note: This could also be something similar to update_data(self, data: DataFrameLikeT), but the required infrastructure code changes would be similar.


In Shiny-R I often use the dataTableProxy() ....

I do not believe a proxy object will be created within py-shiny. However, Python is pass by reference and we can empower our renderers to have extra methods. These extra methods, (e.g. .data_view() or .update_sort() or even .update_data()) should cover the benefits of proxy object.


One open question that I had was "how should the updates be supplied?". Should it be at the cell level or at the "whole data frame" level?

  • Cell
    • Efficient and precise
    • Harder to work with as a user. Must retrieve to row, col, value info for every cell.
  • Whole data frame
    • Inefficient. Will need to send the whole data frame to the browser
    • Comfortable to work with as a user. Keeps the interface transaction as data frame in and data frame out

Thoughts?

@maxmoro
Copy link
Author

maxmoro commented Jul 15, 2024

Thank you for your prompt reply. Here are a couple of examples of common use cases I can think of:

  1. Single Row Edit: A Shiny app displays a list (data frame). The user selects a row and clicks an "Edit" button. A form appears, allowing the user to modify the selected row's information. The form handles the editing logic. When the user clicks "OK," the changes are applied to the row in the list.
  2. Full Table Refresh: The user triggers a refresh or recalculation of the entire table. The table needs to be reloaded from scratch with updated data.

In the first case, editing at the cell level is the most efficient and streamlined approach. The second case requires a full table refresh, so resetting the entire data frame is quicker. (reactive on the @render.data_frame) But the user will lose the filters and sort. (even if the new options in 1.0 will help to reset them)

Based on my experience, I would recommend prioritizing cell-level updates . Whole-table refreshes could be a second priority.

One open question that I had was "how should the updates be supplied?". Should it be at the cell level or at the "whole data frame" level?

  • Cell

    • Efficient and precise
    • Harder to work with as a user. Must retrieve to row, col, value info for every cell.
  • Whole data frame

    • Inefficient. Will need to send the whole data frame to the browser
    • Comfortable to work with as a user. Keeps the interface transaction as data frame in and data frame out

Thoughts?

@maxmoro
Copy link
Author

maxmoro commented Jul 15, 2024

I do not believe a proxy object will be created within py-shiny. However, Python is pass by reference and we can empower our renderers to have extra methods. These extra methods, (e.g. .data_view() or .update_sort() or even .update_data()) should cover the benefits of proxy object.

I fully agree, I think Python's by-reference approach is very useful and easy to code with. I intuitively built an App where the edit of a cell triggers other cells to change, just using the referenced data set (.data() and .data_view()).

@schloerke
Copy link
Collaborator

Currently, when a @render.data_frame function executes these qualities are reset:

  • column filtering
  • column sorting
  • selected rows
  • user edits

I believe not losing these qualities are the root of the issue.

  • Having update_cell_value() would allow us to shim in new values and not reset any of the qualities. (We're in agreement)
  • Having a update_data() method could update the data and accept the qualities as parameters to opt-in/opt-out of being updated. If the qualities are maintained, then we could require certain data characteristics to be consistent between the old and the new data. Seems reasonable to reset all at once and not individual qualities. If anything, their corresponding update method can be run.
  • I currently taking the stance that every time a renderer function runs, all qualities are reset. Having a renderer run as an update would be a new pattern and I'd like to avoid it.

Pseudo code

def update_data(self, data: DataFrameLikeT, *, reset: bool | None = None) -> None:
    if reset is True:
		# everything naturally resets
		...
	else:
		action_type = warning if reset is None else error # error when reset = False
		for each quality, display any error messages with the action_type
			verify new data and old data have same column types
			verify all existing edits and selections are within the bounds of the new data

	Send message to browser with `data` and `reset`

and for completeness

def update_cell_value(self, value: TagNode, *, row: int, col: int) -> None:
    Add cell patch info to internal cell patches dictionary
    It feels off to call the currently set patches method on this submitted value
    client_value = maybe_as_cell_html(value)
	Send message to browser with `value: client_value`, `row`, and `col`

@maxmoro
Copy link
Author

maxmoro commented Jul 15, 2024

I agree with your points. Your pseudo-code would be awesome. It would streamline the process (creation vs. editing vs. refresh data), keep it simple to code, and avoid getting lost in the @render reactivity (in R we need to use Isolate to avoid re-triggering the rendered)
Thanks!

@schloerke schloerke changed the title Equivalent to R dataTableProxy() to manipulate data in the data_frame feat(data_frame): Add .update_data(data, *, reset) and .update_cell_value(value, *, row, col) Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data frame Related to @render.data_frame enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants