Skip to content

Commit

Permalink
Fix dataframe maximum update depth error (streamlit#9490)
Browse files Browse the repository at this point in the history
## Describe your changes

Fixes an issue with the dataframe component that causes random crashes:


![image](https://github.com/user-attachments/assets/da32e051-95ac-4562-aee6-bdb82a2fbd27)

The crash can happen on low-resolution monitors if dataframes are
rendered with `use_container_width=True` are used in some of our
containers (popover, tabs, sidebar, columns). It appears that our width
calculation in the Block component causes this. If the width can't be
determined yet by the resize observer, it can be -1 for some time. This
seems to be a trigger for the maximum update depth. Unfortunately, I
don't have a validated explanation of why it actually leads to the error
:(

## GitHub Issue Link (if applicable)

- Closes streamlit#7949

## Testing Plan

- Add e2e test to discover similar breakage. 

---

**Contribution License Agreement**

By submitting this pull request you agree that all contributions to this
project are made under the Apache 2.0 license.
  • Loading branch information
lukasmasuch authored Sep 19, 2024
1 parent 5622102 commit 7f43ef6
Show file tree
Hide file tree
Showing 5 changed files with 164 additions and 13 deletions.
45 changes: 45 additions & 0 deletions e2e_playwright/st_dataframe_stable_rendering.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# Copyright (c) Streamlit Inc. (2018-2022) Snowflake Inc. (2022-2024)
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

"""This test checks that the dataframe component renders without crashing
when used in different containers.
This mainly addresses this issue: https://github.com/streamlit/streamlit/issues/7949
"""

import numpy as np
import pandas as pd

import streamlit as st

np.random.seed(0)
df = pd.DataFrame(np.random.randn(20, 5), columns=["a", "b", "c", "d", "e"])

use_container_width = st.toggle("use_container_width", True)

with st.popover("popover"):
st.dataframe(df, use_container_width=use_container_width)

with st.sidebar:
st.dataframe(df, use_container_width=use_container_width)

tab1, tab2 = st.tabs(["Tab 1", "Tab 2"])

col1, col2, col3 = tab1.columns([1, 2, 3])
col1.dataframe(df, use_container_width=use_container_width, height=100)
col2.dataframe(df, use_container_width=use_container_width, height=100)
col3.dataframe(df, use_container_width=use_container_width, height=100)

tab1.dataframe(df, use_container_width=use_container_width, height=100)
tab2.dataframe(df, use_container_width=use_container_width, height=100)
46 changes: 46 additions & 0 deletions e2e_playwright/st_dataframe_stable_rendering_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# Copyright (c) Streamlit Inc. (2018-2022) Snowflake Inc. (2022-2024)
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.


from playwright.sync_api import Page, expect

from e2e_playwright.conftest import wait_for_app_loaded
from e2e_playwright.shared.app_utils import click_toggle


def test_dataframe_renders_without_crashing(app: Page):
"""Test that st.dataframe renders without crashing."""

# Reload the page a couple of times to make sure that the dataframe
# crash doesn't appear.
# This test is safeguarding against potential regressions that
# cause crashes as report in: https://github.com/streamlit/streamlit/issues/7949
# But these crashes are usually more random, thats why we run
# it for a couple of page reloads.
# Also, even if there are crashes, its not gurunteed that they will
# happen in our CI environment.
for _ in range(5):
dataframe_elements = app.get_by_test_id("stDataFrame")
expect(dataframe_elements).to_have_count(7)
expect(app.get_by_test_id("stAlertContainer")).not_to_be_attached()

# Set use_container_width to False:
click_toggle(app, "use_container_width")
dataframe_elements = app.get_by_test_id("stDataFrame")
expect(dataframe_elements).to_have_count(7)
expect(app.get_by_test_id("stAlertContainer")).not_to_be_attached()

# Reload the page:
app.reload()
wait_for_app_loaded(app)
2 changes: 1 addition & 1 deletion e2e_playwright/st_popover.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@

with st.popover("popover 4 (with dataframe)", help="help text"):
st.markdown("Popover with dataframe")
st.dataframe(df, use_container_width=False)
st.dataframe(df, use_container_width=True)
st.image(np.repeat(0, 100).reshape(10, 10))

with st.sidebar.popover("popover 5 (in sidebar)"):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,13 @@

import { act, renderHook } from "@testing-library/react-hooks"

import { Arrow as ArrowProto } from "@streamlit/lib/src/proto"
import { TEN_BY_TEN, UNICODE, VERY_TALL } from "@streamlit/lib/src/mocks/arrow"
import { Arrow as ArrowProto } from "@streamlit/lib/src/proto"

import useTableSizer, { calculateMaxHeight } from "./useTableSizer"
import useTableSizer, {
calculateMaxHeight,
MIN_TABLE_WIDTH,
} from "./useTableSizer"

describe("useTableSizer hook", () => {
it("applies the configured width", () => {
Expand All @@ -42,6 +45,29 @@ describe("useTableSizer hook", () => {
expect(result.current.maxWidth).toEqual(CONTAINER_WIDTH)
})

it("Uses the minimum table width if container width is -1", () => {
// The width of the surrounding containers can be -1 in some edge cases
// caused by the resize observer in the Block component.
// We test that the dataframe component correctly handles this case
// by falling back to the minimum table width instead.
// Related to: https://github.com/streamlit/streamlit/issues/7949
const CONTAINER_WIDTH = -1
const { result } = renderHook(() =>
useTableSizer(
ArrowProto.create({
data: TEN_BY_TEN,
useContainerWidth: true,
}),
10,
CONTAINER_WIDTH
)
)

expect(result.current.resizableSize.width).toEqual(MIN_TABLE_WIDTH)
expect(result.current.maxWidth).toEqual(MIN_TABLE_WIDTH)
expect(result.current.minWidth).toEqual(MIN_TABLE_WIDTH)
})

it("adapts to the surrounding container width", () => {
// The width of the surrounding containers
const CONTAINER_WIDTH = 200
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,25 @@ export const BORDER_THRESHOLD = 2
export const ROW_HEIGHT = 35
// Min width for the resizable table container:
// Based on one column at minimum width + borders
const MIN_TABLE_WIDTH = MIN_COLUMN_WIDTH + BORDER_THRESHOLD
export const MIN_TABLE_WIDTH = MIN_COLUMN_WIDTH + BORDER_THRESHOLD
// Min height for the resizable table container:
// Based on header + one column, and border threshold
const MIN_TABLE_HEIGHT = 2 * ROW_HEIGHT + BORDER_THRESHOLD
// The default maximum height of the table:
const DEFAULT_TABLE_HEIGHT = 400

export type AutoSizerReturn = {
// The minimum height that the data grid can be resized to
minHeight: number
// The maximum height of the data grid can be resized to
maxHeight: number
// The minimum width of the data grid can be resized to
minWidth: number
// The maximum width of the data grid can be resized to
maxWidth: number
// The current (or initial) size of the data grid
resizableSize: ResizableSize
// A callback function to change the size of the data grid.
setResizableSize: React.Dispatch<React.SetStateAction<ResizableSize>>
}

Expand All @@ -74,12 +80,18 @@ function useTableSizer(
containerHeight?: number,
isFullScreen?: boolean
): AutoSizerReturn {
// Calculate the maximum height of the table based on the number of rows:
let maxHeight = calculateMaxHeight(
numRows +
1 + // Column header row
(element.editingMode === ArrowProto.EditingMode.DYNAMIC ? 1 : 0) // Trailing row
)

// The initial height is either the default table height or the maximum
// (full) height based if its smaller than the default table height.
// The reason why we have initial height is that the table itself is
// resizable by the user. So, it starts with initial height but can be
// resized between min and max height.
let initialHeight = Math.min(maxHeight, DEFAULT_TABLE_HEIGHT)

if (element.height) {
Expand All @@ -100,22 +112,44 @@ function useTableSizer(
}
}

let initialWidth: number | undefined // If container width is undefined, auto set based on column widths
let maxWidth = containerWidth
// The available width should be at least the minimum table width
// to prevent "maximum update depth exceeded" error. The reason
// is that the container width can be -1 in some edge cases
// caused by the resize observer in the Block component.
// This can trigger the "maximum update depth exceeded" error
// within the grid component.
const availableWidth = Math.max(containerWidth, MIN_TABLE_WIDTH)

// The initial width of the data grid.
// If not set, the data grid will be auto adapted to its content.
// The reason why we have initial width is that the data grid itself
// is resizable by the user. It starts with initial width but can be
// resized between min and max width.
let initialWidth: number | undefined
// The maximum width of the data grid can be resized to.
let maxWidth = availableWidth

if (element.useContainerWidth) {
// Always use the full container width
initialWidth = containerWidth
// If user has set use_container_width,
// use the full container (available) width.
initialWidth = availableWidth
} else if (element.width) {
// User has explicitly configured a width
// The user has explicitly configured a width
// use it but keep between the MIN_TABLE_WIDTH
// and the available width.
initialWidth = Math.min(
Math.max(element.width, MIN_TABLE_WIDTH),
containerWidth
availableWidth
)
maxWidth = Math.min(Math.max(element.width, maxWidth), containerWidth)
// Make sure that the max width we configure is between the user
// configured width and the available (container) width.
maxWidth = Math.min(Math.max(element.width, maxWidth), availableWidth)
}

const [resizableSize, setResizableSize] = React.useState<ResizableSize>({
// If user hasn't specified a width via `width` or `use_container_width`,
// we configure the table to 100%. Which will cause the data grid to
// calculate the best size on the content and use that.
width: initialWidth || "100%",
height: initialHeight,
})
Expand All @@ -125,11 +159,11 @@ function useTableSizer(
// changes and the table uses the full container width.
if (element.useContainerWidth && resizableSize.width === "100%") {
setResizableSize({
width: containerWidth,
width: availableWidth,
height: resizableSize.height,
})
}
}, [containerWidth])
}, [availableWidth])

// Reset the height if the number of rows changes (e.g. via add_rows):
React.useLayoutEffect(() => {
Expand Down

0 comments on commit 7f43ef6

Please sign in to comment.