Fix Pandas styler performance regression#12422
Conversation
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) |
✅ PR preview is ready!
|
Co-authored-by: Copilot <[email protected]>
sfc-gh-gwatters
left a comment
sfc-gh-gwatters
left a comment
There was a problem hiding this comment.
Current change looks good to me - re-request a review if you decide to optimize further!
We should add a perf test that tracks this scenario.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
lukasmasuch
added
security-assessment-completed
change:bugfix
There was a problem hiding this comment.
Pull Request Overview
This PR addresses a performance regression in Pandas styler processing by optimizing data access patterns. The changes replace inefficient .iloc[] accessor usage with faster .iat[] accessor for single-element access and implement batch updates for display value assignments.
Key Changes:
- Replace
.iloc[0]with.iat[0]for single-element access across multiple files - Refactor pandas styler display value updates to use batch assignment instead of individual cell updates
- Optimize color column processing in chart utilities
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| lib/streamlit/elements/map.py | Replace .iloc[0] with .iat[0] for faster single-element access in color column validation |
| lib/streamlit/elements/lib/pandas_styler_utils.py | Major refactor to batch display value updates instead of individual .iloc[] assignments |
| lib/streamlit/elements/lib/built_in_chart_utils.py | Replace .iloc[0] with .iat[0] in three locations for date and color column processing |
sfc-gh-lmasuch
changed the title
|
@sfc-gh-gwatters I finished up the PR and also applied the suggested performance improvement + added a performance benchmark. |
|
Looks good. Did you verify that my suggestion was any faster? |
yep, it's consistently faster, gets it down to 1.5s for the example :) |
Describe your changes
The change to use
ilocinstead ofiatapplied in #10571 caused performance issues. This PR fixes the performance issues by using a more efficient way to update the values in the dataframe.GitHub Issue Link (if applicable)
Testing Plan
Contribution License Agreement
By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.