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

Ensure row numbers functionality in dataframe works as expected #10490

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

hannahblair
Copy link
Collaborator

@hannahblair hannahblair commented Feb 3, 2025

Description

This PR:

  • Adds the show_row_numbers param which seemed to get lost in a merge
  • Allows you to sort by row numbers in the data frame component
  • Updates the sort by icon. let me know what you think; I don't love the other arrow that we use (I'm aware we use this elsewhere though)

Kapture 2025-02-03 at 17 42 54

Closes: #10411

🎯 PRs Should Target Issues

Before your create a PR, please check to see if there is an existing issue for this change. If not, please create an issue before you create this PR, unless the fix is very small.

Not adhering to this guideline will result in the PR being closed.

Testing and Formatting Your Code

  1. PRs will only be merged if tests pass on CI. We recommend at least running the backend tests locally, please set up your Gradio environment locally and run the backed tests: bash scripts/run_backend_tests.sh

  2. Please run these bash scripts to automatically format your code: bash scripts/format_backend.sh, and (if you made any changes to non-Python files) bash scripts/format_frontend.sh

- tidy up header col spacing
- change sort icon
@gradio-pr-bot
Copy link
Collaborator

gradio-pr-bot commented Feb 3, 2025

🪼 branch checks and previews

Name Status URL
Spaces ready! Spaces preview
Storybook ready! Storybook preview
🦄 Changes detecting...

Install Gradio from this PR

pip install https://gradio-pypi-previews.s3.amazonaws.com/74655791ea67f4901fd887e659a5317e5af5231f/gradio-5.14.0-py3-none-any.whl

Install Gradio Python Client from this PR

pip install "gradio-client @ git+https://github.com/gradio-app/gradio@74655791ea67f4901fd887e659a5317e5af5231f#subdirectory=client/python"

Install Gradio JS Client from this PR

npm install https://gradio-npm-previews.s3.amazonaws.com/74655791ea67f4901fd887e659a5317e5af5231f/gradio-client-1.10.0.tgz

Use Lite from this PR

<script type="module" src="https://gradio-lite-previews.s3.amazonaws.com/74655791ea67f4901fd887e659a5317e5af5231f/dist/lite.js""></script>

@gradio-pr-bot
Copy link
Collaborator

gradio-pr-bot commented Feb 3, 2025

🦄 change detected

This Pull Request includes changes to the following packages.

Package Version
@gradio/core patch
@gradio/dataframe patch
gradio patch
  • Maintainers can select this checkbox to manually select packages to update.

With the following changelog entry.

Ensure row numbers functionality in dataframe works as expected

Maintainers or the PR author can modify the PR title to modify this entry.

Something isn't right?

  • Maintainers can change the version label to modify the version bump.
  • If the bot has failed to detect any changes, or if this pull request needs to update multiple packages to different versions or requires a more comprehensive changelog entry, maintainers can update the changelog file directly.

@abidlabs abidlabs requested a review from hysts February 3, 2025 19:26
@abidlabs
Copy link
Member

abidlabs commented Feb 3, 2025

Thanks @hannahblair, this looks great to me! A couple of nits, I'll note below, but which are non-blocking. I'm also cc-ing @hysts for his review

@abidlabs
Copy link
Member

abidlabs commented Feb 3, 2025

  1. It looks like adding show_row_numbers=True still affects the width of the first column. I know you are addressing widths separately, so non-blocking, but here's what I'm seeing:
image

Repro:

import gradio as gr
import numpy as np

np.random.seed(0)
data = np.random.randint(0, 100, size=(10, 10))

with gr.Blocks() as demo:
    gr.Dataframe(value=data, show_row_numbers=True)
demo.launch()
  1. Really like the new icons, including the ability to reverse the dataframe by sorting the column numbers backwards and the animation flourish that happens when you reverse a column sort. My only suggestion would be to indicate the directionality of the sort by only highlighting the up arrow or the down array depending on whether the columns are sorted ascendingly / descendingly:
image

@hannahblair
Copy link
Collaborator Author

hannahblair commented Feb 4, 2025

@abidlabs I've removed the fixed width on the row number header and I've also changed the sort icon to have corresponding up and down coloured arrows. Let me know if that looks good to you!

@abidlabs
Copy link
Member

abidlabs commented Feb 4, 2025

Thanks @hannahblair!

image

Yes, the updates look great! Still have a couple of minor suggestions:

  • The row number column is pretty wide, would be better to make it narrower
  • If you click on the sort icon three times, it resets to the original unclicked state. Does that make sense semantically? I mean the column is still sorted so perhaps you shouldn't be able to unselect the sort icon by clicking on it -- only if you click a different column should the previous column be "unsorted"

Copy link
Collaborator

@hysts hysts left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks @hannahblair

+1 for both two points @abidlabs mentioned above.

Also, regarding the second point, would it be possible to split the sort icon into two separate icons? I've always found it a bit inconvenient to have to press the arrow icon twice to sort in descending order. It would be more convenient if pressing the up arrow sorted in ascending order, pressing the down arrow sorted in descending order, and pressing the currently selected button again reverted to the original order.

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.

show_row_numbers in gr.Dataframe doesn't work
4 participants