Skip to content

Conversation

@mattrunyon
Copy link
Collaborator

Tested with this code (mostly to test Ctrl+a with a large table doesn't hang)

I would not recommend sorting this table as it almost instantly OOM my 20GB DH instance

from deephaven import ui, empty_table

t = ui.table(
    empty_table(1_000_000_000).update_view(["I=ii", "J=I*2"]),
    on_selection_change=lambda d: print(d),
    always_fetch_columns="I"
)

@mattrunyon mattrunyon requested a review from mofojed June 24, 2025 15:53
@mattrunyon mattrunyon self-assigned this Jun 24, 2025
@github-actions github-actions bot requested a review from margaretkennedy June 24, 2025 15:53
@mattrunyon mattrunyon changed the title DH-19292: Add ui.table selection event feat: DH-19292 Add ui.table selection event Jun 24, 2025
@github-actions
Copy link

ui docs preview (Available for 14 days)

@github-actions
Copy link

ui docs preview (Available for 14 days)

@github-actions
Copy link

ui docs preview (Available for 14 days)

@github-actions
Copy link

ui docs preview (Available for 14 days)

mofojed
mofojed previously approved these changes Jun 30, 2025
Copy link
Member

@mofojed mofojed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good - my only comment is if we should bother throwing if always_fetch_columns isn't provided as well.

on_cell_double_press: CellPressCallback | None = None,
on_column_press: ColumnPressCallback | None = None,
on_column_double_press: ColumnPressCallback | None = None,
on_selection_change: SelectionChangeCallback | None = None,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should throw if on_selection_change is set and always_fetch_columns is not set. Right now it just returns empty data if you don't specify always_fetch_columns. Though I guess in theory that would still tell you want the size of the selection is, and maybe that's all the want to know?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ya I added it to the Python side. Throws immediately if not wrapped in a component. If in a component, it will propagate to the client as a render error so the user can see something in that case still.

Did spot a weird case where I ran working code followed by code that threw and the panel just disappeared. I'll investigate a bit more and file a ticket

@mattrunyon mattrunyon dismissed stale reviews from mofojed and margaretkennedy via 792cc08 June 30, 2025 21:28
@mattrunyon mattrunyon requested a review from mofojed June 30, 2025 21:28
@github-actions
Copy link

ui docs preview (Available for 14 days)

@mattrunyon mattrunyon merged commit 85db04b into deephaven:main Jul 2, 2025
28 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants