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

Improve performance on maps with many children #142

Merged
merged 11 commits into from
Sep 22, 2023
20 changes: 18 additions & 2 deletions .github/workflows/run_tests_each_PR.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,22 @@ jobs:
- name: Install playwright dependencies
run: |
playwright install --with-deps
- name: Test with pytest
- name: Install annotate-failures-plugin
run: pip install pytest-github-actions-annotate-failures

- name: Test with pytest and retry flaky tests up to 3 times
run: |
pytest --browser chromium -s
pytest --browser chromium -s --reruns 3 --junit-xml=test-results.xml

- name: Surface failing tests
if: always()
uses: pmeier/pytest-results-action@main
with:
path: test-results.xml
fail-on-empty: false

- uses: actions/upload-artifact@v3
if: failure()
with:
name: screenshots
path: screenshot*.png
4 changes: 3 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -76,4 +76,6 @@ package-lock.json
.direnv/
.envrc

.streamlit/
.streamlit/

test-results.xml
70 changes: 49 additions & 21 deletions examples/pages/misc_examples.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,30 +15,58 @@

# center on Liberty Bell, add marker
if page == "Single map":
with st.echo():
m = folium.Map(location=[39.949610, -75.150282], zoom_start=16)
tooltip = "Liberty Bell"
folium.Marker(
[39.949610, -75.150282], popup="Liberty Bell", tooltip=tooltip
).add_to(m)
m = folium.Map(location=[39.949610, -75.150282], zoom_start=16)
tooltip = "Liberty Bell"
folium.Marker(
[39.949610, -75.150282], popup="Liberty Bell", tooltip=tooltip
).add_to(m)
st.code(
"""
m = folium.Map(location=[39.949610, -75.150282], zoom_start=16)
tooltip = "Liberty Bell"
folium.Marker(
[39.949610, -75.150282], popup="Liberty Bell", tooltip=tooltip
).add_to(m)
""",
language="python",
)

elif page == "Dual map":
with st.echo():
m = folium.plugins.DualMap(location=[39.949610, -75.13], zoom_start=16)
tooltip = "Liberty Bell"
folium.Marker(
[39.949610, -75.150282], popup="Liberty Bell", tooltip=tooltip
).add_to(m)

m = folium.plugins.DualMap(location=[39.949610, -75.13], zoom_start=16)
tooltip = "Liberty Bell"
folium.Marker(
[39.949610, -75.150282], popup="Liberty Bell", tooltip=tooltip
).add_to(m)
st.code(
"""
m = folium.plugins.DualMap(location=[39.949610, -75.13], zoom_start=16)
tooltip = "Liberty Bell"
folium.Marker(
[39.949610, -75.150282], popup="Liberty Bell", tooltip=tooltip
).add_to(m)
""",
language="python",
)
else:
with st.echo():
m = branca.element.Figure()
fm = folium.Map(location=[39.949610, -75.150282], zoom_start=16)
tooltip = "Liberty Bell"
folium.Marker(
[39.949610, -75.150282], popup="Liberty Bell", tooltip=tooltip
).add_to(fm)
m.add_child(fm)
m = branca.element.Figure()
fm = folium.Map(location=[39.949610, -75.150282], zoom_start=16)
tooltip = "Liberty Bell"
folium.Marker(
[39.949610, -75.150282], popup="Liberty Bell", tooltip=tooltip
).add_to(fm)
m.add_child(fm)
st.code(
"""
m = branca.element.Figure()
fm = folium.Map(location=[39.949610, -75.150282], zoom_start=16)
tooltip = "Liberty Bell"
folium.Marker(
[39.949610, -75.150282], popup="Liberty Bell", tooltip=tooltip
).add_to(fm)
m.add_child(fm)
""",
language="python",
)

with st.echo():
# call to render Folium map in Streamlit
Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

setuptools.setup(
name="streamlit_folium",
version="0.13.0",
version="0.14.0",
author="Randy Zwitch",
author_email="[email protected]",
description="Render Folium objects in Streamlit",
Expand Down
18 changes: 16 additions & 2 deletions streamlit_folium/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,21 @@ def _generate_leaflet_string(
return leaflet, mappings


_FOLIUM_VAR_SUFFIX_PATTERN = re.compile("_[a-z0-9]+(?!_)")


def _replace_folium_vars(leaflet: str, mappings: dict[str, str]) -> str:
def replace(match: re.Match):
match_str = match.group()
leaflet_id = match_str.strip("_")
replacement = mappings.get(leaflet_id)
if replacement:
match_str = match_str.replace(leaflet_id, replacement)
return match_str

return _FOLIUM_VAR_SUFFIX_PATTERN.sub(replace, leaflet)


def generate_leaflet_string(
m: folium.MacroElement, nested: bool = True, base_id: str = "div"
) -> str:
Expand All @@ -389,7 +404,6 @@ def generate_leaflet_string(
"""
leaflet, mappings = _generate_leaflet_string(m, nested=nested, base_id=base_id)

for k, v in mappings.items():
leaflet = leaflet.replace(k, v)
leaflet = _replace_folium_vars(leaflet, mappings)

return leaflet
3 changes: 2 additions & 1 deletion tests/requirements.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
streamlit>1.11.1
pytest>=7.1.2
folium>=0.13
pytest-playwright
pytest-playwright
pytest-rerunfailures
46 changes: 33 additions & 13 deletions tests/test_frontend.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,15 @@
@pytest.fixture(scope="function", autouse=True)
def before_test(page: Page):
page.goto(f"localhost:{PORT}")
page.set_viewport_size({"width": 2000, "height": 2000})


# Take screenshot of each page if there are failures for this session
@pytest.fixture(scope="function", autouse=True)
def after_test(page: Page, request):
yield
if request.node.rep_call.failed:
page.screenshot(path=f"screenshot-{request.node.name}.png", full_page=True)


@contextmanager
Expand Down Expand Up @@ -59,9 +68,13 @@
expect(page.get_by_text('"last_object_clicked":NULL')).to_be_visible()

# Click marker
page.frame_locator(
'internal:attr=[title="streamlit_folium.st_folium"i]'
).get_by_role("img").nth(0).click()
try:
page.frame_locator('iframe[title="streamlit_folium\\.st_folium"]').get_by_role(
"img"
).click()
except Exception as e:
page.screenshot(path="screenshot-test-marker-click.png", full_page=True)
raise e

expect(page.get_by_text('"last_object_clicked":NULL')).to_be_hidden()

Expand Down Expand Up @@ -110,17 +123,20 @@

expect(page).to_have_title("streamlit-folium documentation: Misc Examples")

page.locator("label").filter(has_text="Dual map").click()
page.locator("label").filter(has_text="Dual map").click()

# Click marker on left map
page.frame_locator('internal:attr=[title="streamlit_folium.st_folium"i]').locator(
"#map_div"
).get_by_role("img").nth(0).click()

# Click marker on right map
page.frame_locator('internal:attr=[title="streamlit_folium.st_folium"i]').locator(
"#map_div2"
).get_by_role("img").nth(0).click()
try:
page.frame_locator('iframe[title="streamlit_folium\\.st_folium"]').locator(
"#map_div"
).get_by_role("img").click()
page.frame_locator('iframe[title="streamlit_folium\\.st_folium"]').locator(
"#map_div2"
).get_by_role("img").click()
except Exception as e:
page.screenshot(path="screenshot-dual-map.png", full_page=True)
raise e


def test_vector_grid(page: Page):
Expand Down Expand Up @@ -190,15 +206,17 @@
def test_responsiveness(page: Page):
page.get_by_role("link", name="responsive").click()

page.set_viewport_size({"width": 500, "height": 1000})
page.set_viewport_size({"width": 500, "height": 3000})

initial_bbox = (
page.frame_locator("div:nth-child(2) > iframe")

Check failure on line 212 in tests/test_frontend.py

View workflow job for this annotation

GitHub Actions / build (3.9)

test_responsiveness[chromium] playwright._impl._api_types.TimeoutError: Timeout 30000ms exceeded. =========================== logs =========================== waiting for frame_locator("div:nth-child(2) > iframe").locator("#map_div") ============================================================
.locator("#map_div")
.bounding_box()
)

page.set_viewport_size({"width": 1000, "height": 1000})
page.set_viewport_size({"width": 1000, "height": 3000})

sleep(1)

new_bbox = (
page.frame_locator("div:nth-child(2) > iframe")
Expand All @@ -214,3 +232,5 @@
assert new_bbox is not None

assert new_bbox["width"] > initial_bbox["width"] + 300

page.set_viewport_size({"width": 2000, "height": 2000})
Loading