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

Add additional selection modes #3725

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

nojaf
Copy link

@nojaf nojaf commented Feb 8, 2025

📝 Summary

Fixes #3624

🔍 Description of Changes

This is a WIP, but I started with adding the new selection modes to both the JavaScript and Python side.

I still need to to the actual implementation, but since I'm new to this project I could benefit from some early feedback.

Open questions:

  • Where should the checkbox in the cell be positioned? What do you folks have in mind here UI-wise?
  • Should initial_selection be taking into account here? Or would that remain row specific?

📋 Checklist

📜 Reviewers

@mscolnick

Copy link

vercel bot commented Feb 8, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
marimo-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 19, 2025 2:34pm
marimo-storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 19, 2025 2:34pm

Copy link

github-actions bot commented Feb 8, 2025

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@nojaf
Copy link
Author

nojaf commented Feb 8, 2025

I have read the CLA Document and I hereby sign the CLA

def select_cells(self, cells: list[Cell]) -> TableManager[Any]:
df = self.as_frame()
# This does not work and I could use some help
return self.with_new_data([df.at[row, col] for row, col in cells])
Copy link
Author

@nojaf nojaf Feb 19, 2025

Choose a reason for hiding this comment

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

Hello @mscolnick, any ideas what I need to change here?
I'm a bit confused what exactly should be returned here.
This code doesn't work because of narwhal stuff

image

But I'm also unclear if I need this.

Because I'm unsure how this would play out if the on_change callback?

Copy link
Contributor

Choose a reason for hiding this comment

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

this function currently expect a TableManager back, which takes in a dataframe, but you are giving it a list.

we could:

  1. give return self.with_new_data( a dataframe instead of a list
  2. change def select_cells(self, cells: list[Cell]) -> TableManager[Any]: to def select_cells(self, cells: list[Cell]) -> list[CellWithValue]:

if you do go with 2. Instead of CellWithValue. maybe add an optional value to the Cell class.

for implementation:

  1. you might want to filter to just the subset of all columns and all rows used, then convert to a dictionary, then pick out the correct row/col pairs, and then back to a dataframe
  2. this is likely easier (same implementation you currently have), but may affect downstream change such as exporting

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.

Propogate column information when cell selected in mo.ui.Table
2 participants