-
Notifications
You must be signed in to change notification settings - Fork 10
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/st dataframe pagenation #77
Conversation
PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here. PR Reviewer Guide 🔍
|
PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here. PR Code Suggestions ✨
|
Thanks a lot for the addition. I will review this PR early next week as I am OOO for the rest of this week. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the addition! I just have some minor comments. After those are addressed the PR would be good to merge from my side.
src/common.py
Outdated
|
||
event = st.dataframe( | ||
current_chunk_df, | ||
column_order=[ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be more flexible as the data displayed may vary. I think adding column_order
as a function parameter would work best. Is there a reason why selection_mode
, on_select
, use_container_width
, and hide_index
have to be set to these specific values? If not I would suggest adding those as parameters as well.
Anything specific to a particular workflow/dataset should not be located in common.py
as it is intended to contain functions of general use. As a sidenote, common modules should be moved in a separate directory to make this more clear to developers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point. These are specifically set because I came across this issue when using the pyopenms workflow for selecting spectra to plot when selecting a row, but this should be more flexible for other use cases.
We could pass kwargs
to the display_large_dataframe
method for st.dataframe
because that's probably the only main method that would need to be changed based on particular needs. I opted for this option
Merging with |
Thanks for making the changes. I like the solution with |
User description
This add a method for rendering a streamlit dataframe with pagenation to allow viewing small chunks of the dataframe at a time, which also solves #76
PR Type
enhancement
Description
display_large_dataframe
insrc/common.py
to render large DataFrames with pagination and row selection.st.dataframe
call insrc/view.py
with the newdisplay_large_dataframe
method to enhance theview_spectrum
functionality.Changes walkthrough 📝
common.py
Add paginated DataFrame display and memory usage calculation
src/common.py
display_large_dataframe
for paginated DataFramedisplay.
view.py
Integrate paginated DataFrame display in view_spectrum
src/view.py
st.dataframe
call withdisplay_large_dataframe
.view_spectrum
.