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(df): Make .input_cell_selection() return a consistent type shape; Change row order to data view row order #1376

Merged
merged 8 commits into from
May 21, 2024

Conversation

schloerke
Copy link
Collaborator

@schloerke schloerke commented May 14, 2024

Should be rebased after #1374 is merged!

TODO

** Update merging into #1374 to get changes


Features for .input_cell_selection():

  • Removed None return type
  • Added missing cols (or rows) when the input object sent across the wire does not contain the values.
  • Row order is now ordered as if it is being subsetted from Data View row order

Note:
input.<ID>_cell_selection()'s shape did not change. It does not contain missing cols (or rows) as that information is always unnecessary.

(However, this leads to an awkward user interaction with the code, so <ID>.input_cell_selection() upgrades the return value to support the missing keys.)


Previous return type for .input_cell_selection():

class CellSelectionNone(TypedDict):
    type: Literal["none"]

class CellSelectionRow(TypedDict):
    type: Literal["row"]
    rows: ListOrTuple[int]

class CellSelectionCol(TypedDict):
    type: Literal["col"]
    cols: ListOrTuple[int]

class CellSelectionRect(TypedDict):
    type: Literal["rect"]
    # Attempt to type the list of size 2, but it is not type enforced
    rows: tuple[int, int] | Annotated[list[int], 2]
    cols: tuple[int, int] | Annotated[list[int], 2]

CellSelection = Union[
    CellSelectionRow,
    CellSelectionCol,
    CellSelectionRect,
    CellSelectionNone,
]

def .input_cell_selection() -> CellSelection | None:

New return type for .input_cell_selection():

class CellSelection(TypedDict):
    type: Literal["none", "row", "col", "rect"]
    rows: ListOrTuple[int]
    cols: ListOrTuple[int]

def .input_cell_selection() -> CellSelection:
  • If type = "none", rows and cols are empty tuples.
  • If type = "row", cols is all column numbers of the data.
  • If type = "col", rows is all row numbers of the data view rows
  • If type = "rect", all rows and cols are provided (no longer just min/max values).

Previous user interaction:

@render.data_frame
def grid(): ...

selected_rows = (grid.input_cell_selection() or {}).get("rows", ())

New user interaction:

selected_rows = grid.input_cell_selection()["rows"]

@schloerke schloerke added this to the v0.9.1 milestone May 14, 2024
@schloerke schloerke requested review from karangattu and removed request for karangattu May 14, 2024 16:02
@schloerke schloerke added the data frame Related to @render.data_frame label May 14, 2024
@schloerke schloerke requested a review from wch May 14, 2024 16:09
@schloerke schloerke marked this pull request as ready for review May 14, 2024 16:09
@wch
Copy link
Collaborator

wch commented May 14, 2024

Note that after we release this, we'll need to update the examples in py-shiny-site. The changes would be a follow-up to posit-dev/py-shiny-site#143.

@schloerke schloerke marked this pull request as draft May 14, 2024 17:55
@schloerke schloerke modified the milestones: v0.9.1, v0.10.0 May 14, 2024
@schloerke schloerke changed the title feat(df): Make .input_cell_selection() return a consistent type shape feat(df): Make .input_cell_selection() return a consistent type shape; Change row order to data view row order May 20, 2024
@schloerke
Copy link
Collaborator Author

schloerke commented May 20, 2024

From #1374 (comment)

  • Also, input_cell_selection rows should be ordered the way the user sees it.
  • In the meeting, we also came to the agreement that the None return type should be reinstated so that it is handled by the user

* data_view_meta:
  Add demo apps for `update_sort()` and `update_filter()`
  Add `update_sort()` and `update_filter()` to DF
  bug(output transformer): fix transformer auto-registration (#1394)
  fix: Add wait till pulse animation has started (#1393)
  bug(test-deploy): Add retries for deploy tests (#1392)
  Add busy indicator tests (#1391)
  Yield to give "synchronous" writes a chance to complete (#1388)
  chore(busy indicator): Update busy indicator css files (#1389)
  `ColumnFilter` and `ColumnSort` should use `col: num` and not `id: str` for consistency
  Lints
  Have `.data_view()` use `.data_view_info()` information for consistent subsetting
  feat(cli): Add `shiny --version` (#1387)
  fix(selectize): Accept jsonifiable values in `options` dictionary (#1382)
  bug(data frame): Use `<ID>_data_view_rows` (#1386)
  test(data frame): Verify that data frame's outputs are reset before moving forward (#1383)
  Send busy/idle at the right times (#1380)
  feat(data frame): Restore `input.<ID>_selected_rows()`. Rename `input.<ID>_data_view_indices` to `input.<ID>_data_view_rows` (#1377)
  Apply suggestions from code review
@schloerke
Copy link
Collaborator Author

@jcheng5 None support within .input_cell_selection() feels wonky. Should it instead throw a req(False)?

* data_view_meta:
  `.input_column_sort()` -> `.input_sort()`; `.input_column_filter()` -> `.input_filter()`
  Add type support for `serialize_numpy_dtype()`; Determine default sorting direction
  Update controls.py
  Remove `.data_view_info()` method
  Add `.data_view_rows()`
@schloerke schloerke marked this pull request as ready for review May 21, 2024 17:32
@schloerke
Copy link
Collaborator Author

Squash merging into base branch for easier PR interactions as main is frozen for v0.10.0 release

@schloerke schloerke merged commit 1a07fe0 into data_view_meta May 21, 2024
3 of 30 checks passed
@schloerke schloerke deleted the df_input_cell_selection branch May 21, 2024 17:34
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants