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

Allow freezing columns in gr.Dataframe #10561

Merged
merged 27 commits into from
Feb 11, 2025
Merged

Allow freezing columns in gr.Dataframe #10561

merged 27 commits into from
Feb 11, 2025

Conversation

hannahblair
Copy link
Collaborator

@hannahblair hannahblair commented Feb 10, 2025

Description

Adds a frozen_cols param which freezes up to X number of columns in the dataframe.

Note, It'd be cool to be able to freeze columns at runtime but some design considerations need to be made first.

Closes: #2407

🎯 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

@gradio-pr-bot
Copy link
Collaborator

gradio-pr-bot commented Feb 10, 2025

🪼 branch checks and previews

Name Status URL
Spaces ready! Spaces preview
Website ready! Website preview
Storybook ready! Storybook preview
🦄 Changes detected! Details

Install Gradio from this PR

pip install https://gradio-pypi-previews.s3.amazonaws.com/215445f3795567f5f23dcdfbdaaf6ff8ff6cc4ca/gradio-5.15.0-py3-none-any.whl

Install Gradio Python Client from this PR

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

Install Gradio JS Client from this PR

npm install https://gradio-npm-previews.s3.amazonaws.com/215445f3795567f5f23dcdfbdaaf6ff8ff6cc4ca/gradio-client-1.11.0.tgz

Use Lite from this PR

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

@gradio-pr-bot
Copy link
Collaborator

gradio-pr-bot commented Feb 10, 2025

🦄 change detected

This Pull Request includes changes to the following packages.

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

With the following changelog entry.

Allow freezing columns in gr.Dataframe

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.

}

const cell_rect = cell_el.getBoundingClientRect();
const table_rect = table.getBoundingClientRect();

const col_pos = `${cell_rect.left - table_rect.left + cell_rect.width / 2}px`;
Copy link
Collaborator Author

@hannahblair hannahblair Feb 10, 2025

Choose a reason for hiding this comment

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

Use the actual cell position from the DOM instead of trying to calculate it from CSS variables. I've changed this logic because when using custom column widths via column_widths, I noticed that the column selection button wasn't positioned properly

@hannahblair hannahblair marked this pull request as ready for review February 10, 2025 21:47
@@ -67,6 +67,13 @@
export let show_copy_button = false;
export let value_is_output = false;
export let max_chars: number | undefined = undefined;
export let frozen_cols = 0;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

main changes here are adding some frozen CSS classes and always freezing the row number column if it's present

@abidlabs
Copy link
Member

Nice @hannahblair looks really good to me! Just two small issues:

(1) The frozen columns don't have the alternating row shading:

image

(2) It looks like the width logic has been affected, you'll notice that the first three columns are much wider than the other columns.

Screenshot 2025-02-10 at 2 11 35 PM

Repro:

import numpy as np

import gradio as gr

def transpose(matrix):
    return matrix.T

demo = gr.Interface(
    transpose,
    gr.Dataframe(type="numpy", datatype="number", row_count=5, col_count=3, show_fullscreen_button=True, frozen_cols=4),
    "numpy",
    examples=[
        [np.zeros((3, 3)).tolist()],
        [np.ones((2, 2)).tolist()],
        [np.random.randint(0, 10, (3, 10)).tolist()],
        [np.random.randint(0, 10, (10, 3)).tolist()],
        [np.random.randint(0, 10, (10, 10)).tolist()],
    ],
    cache_examples=False
)

if __name__ == "__main__":
    demo.launch()

@hannahblair
Copy link
Collaborator Author

hannahblair commented Feb 10, 2025

@abidlabs both issues addressed! the non alternating background was intentional for frozen columns but I think it's fine without the design change actually. I may readdress the UX when I implement custom frozen cols set by users

edit: actually the widths still arent working ⌛

@pngwn
Copy link
Member

pngwn commented Feb 11, 2025

Two quick things. Can we change this to 'static' or 'pinned' or 'fixed', frozen doesn't really make much sense to me.

Another thing is the API, do we need to limit this to the first x columns. Someone might want to pin the second column or the 2nd and 4th, or the last. I think that should be possible. Even if we don't support this right now, if we think we might want to we should use an api that allows for it. A list of column indices for example.

@hannahblair
Copy link
Collaborator Author

@pngwn

Two quick things. Can we change this to 'static' or 'pinned' or 'fixed', frozen doesn't really make much sense to me.

Not against this, pinned makes most sense to me.

Someone might want to pin the second column or the 2nd and 4th, or the last. I think that should be possible.

Hmm good point. We could plan to allow for either a number (to indicate up to X cols) or an array of ints which represent the col indices? Should be fairly straightforward but needs a bit more time.

@abidlabs
Copy link
Member

Not against this, pinned makes most sense to me

+1

Hmm good point. We could plan to allow for either a number (to indicate up to X cols) or an array of ints which represent the col indices? Should be fairly straightforward but needs a bit more time.

Yeah we can support an int for now to represent the first n columns and then allow support for a list when we add support for pinning arbitrary columns

@aliabid94
Copy link
Collaborator

Two quick things. Can we change this to 'static' or 'pinned' or 'fixed', frozen doesn't really make much sense to me

FYI "frozen" is actually the term google sheets and excel use.

we need to limit this to the first x columns. Someone might want to pin the second column or the 2nd and 4th, or the last. I think that should be possible.

I've never seen this before, google sheets and excel don't support this either. If we really want to add this, we could support a list of indices later, but I think the current way of just an integer representing the first n columns should be preserved.

@freddyaboulton
Copy link
Collaborator

Yes I think I prefer frozen as well since that's the term I'm familiar with.

The API looks good and works as I would expect. One thing I noticed (possibly not related to this PR though) is that column_widths prevents you from horizontally scrolling a dataframe at all. See the mini_leaderboard demo.

@hannahblair
Copy link
Collaborator Author

@freddyaboulton thanks for testing! yeah column_widths are broken completely atm, I can see what you're seeing in main. we've got issue #8099 open to fix it but I'll note in the issue that you can't even scroll when they're set 😅

Copy link
Collaborator

@aliabid94 aliabid94 left a comment

Choose a reason for hiding this comment

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

LGTM!

@hannahblair hannahblair merged commit 26494ce into main Feb 11, 2025
23 checks passed
@hannahblair hannahblair deleted the df-frozen-cols branch February 11, 2025 22:29
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.

Freeze DataFrame columns
6 participants