diff --git a/.github/workflows/linting.yml b/.github/workflows/linting.yml new file mode 100644 index 0000000..6c3027d --- /dev/null +++ b/.github/workflows/linting.yml @@ -0,0 +1,19 @@ +# This workflow is used to run the pre-commit checks on the codebase, particularly +# linting and formatting. + +name: Linting, formatting, and type checking + +on: + push: + branches: [master] + pull_request: + branches: [master] + +jobs: + build: + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + - uses: pre-commit/action@v3.0.1 diff --git a/.github/workflows/run_tests_each_PR.yml b/.github/workflows/run_tests_each_PR.yml index e1d4fbb..50b8238 100644 --- a/.github/workflows/run_tests_each_PR.yml +++ b/.github/workflows/run_tests_each_PR.yml @@ -1,4 +1,4 @@ -# This workflow will install Python dependencies, run tests and lint with a variety of Python versions +# This workflow will install Python dependencies and run tests with a variety of Python versions # For more information see: https://help.github.com/actions/language-and-framework-guides/using-python-with-github-actions name: Run tests each PR @@ -14,7 +14,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - python-version: [3.8, 3.9, "3.10", 3.11, 3.12] + python-version: ["3.9", "3.10", "3.11", "3.12"] steps: - uses: actions/checkout@v3 @@ -33,10 +33,8 @@ jobs: - name: Install python dependencies run: | python -m pip install --upgrade pip - pip install flake8 pytest pytest-playwright - if [ -f tests/requirements.txt ]; then pip install -r tests/requirements.txt; fi + pip install -r tests/requirements.txt pip install -e . - - uses: pre-commit/action@v2.0.3 - name: Install playwright dependencies run: | playwright install --with-deps diff --git a/.gitignore b/.gitignore index 5f3f1e7..d6079c1 100644 --- a/.gitignore +++ b/.gitignore @@ -78,4 +78,6 @@ package-lock.json .streamlit/ .venv/ -test-results.xml \ No newline at end of file +test-results.xml + +.task/ \ No newline at end of file diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 2f04590..a2ed1be 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -5,7 +5,7 @@ repos: hooks: # Run the linter. - id: ruff - args: [--fix] + args: [--fix, --unsafe-fixes] # Run the formatter. - id: ruff-format - repo: https://github.com/pre-commit/mirrors-mypy diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..15293de --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,47 @@ +# Contributing to streamlit-folium + +All of the necessary commands to get the project running are in the +[Taskfile](https://taskfile.dev/). + +### Linting and formatting + +We use [Ruff](https://github.com/astral-sh/ruff) for linting and formatting, and +[mypy](https://github.com/python/mypy) for type checking. + +To run ruff and mypy, you can use the following command: + +```bash +task lint +``` + +### Running tests + +To run the tests, you can use the following command: + +```bash +task test +``` + +### Adding tests + +If you add a new feature, or fix a bug, please add a test to ensure that the feature works as expected. + +The pattern for creating tests is to use the example streamlit app, and use +[playwright](https://playwright.dev/python/docs/intro) to create and +run tests on the app. + +These tests are primarily located in the `tests/frontend.py` file. + +If you are making a change that only affects the python code, you can +run the playwright [codegen](https://playwright.dev/python/docs/codegen) tool to +help generate the tests by running `task generate-tests`. + +If you are making a change that affects the javascript code, you need to set up +folium to use your local frontend code. This can be done by: + +1. Edit `streamlit_folium/__init__.py` to set `_RELEASE = False` +2. Run `task generate-tests-frontend` +3. Add tests as appropriate in `tests/frontend.py` +4. Run `task test-frontend` to run the tests + * Note that `test_release` will fail while `_RELEASE = False` -- this is expected +5. Set `_RELEASE = True` in `streamlit_folium/__init__.py` before opening a PR \ No newline at end of file diff --git a/README.md b/README.md index 1e23146..1cdb186 100644 --- a/README.md +++ b/README.md @@ -36,3 +36,6 @@ Currently, there are two functions defined: ![streamlit_folium example](https://raw.githubusercontent.com/randyzwitch/streamlit-folium/master/tests/visual_baseline/test_basic/first_test/baseline.png) +## Contributing + +See [CONTRIBUTING.md](CONTRIBUTING.md) for more details. \ No newline at end of file diff --git a/Taskfile.yml b/Taskfile.yml new file mode 100644 index 0000000..632af2e --- /dev/null +++ b/Taskfile.yml @@ -0,0 +1,65 @@ +version: "3" + +tasks: + install-pre-commit: + desc: Install pre-commit hooks + cmds: + - pip install pre-commit + - pre-commit install + sources: + - .pre-commit-config.yaml + + install-package: + desc: Install the package + cmds: + - pip install -e . + sources: + - requirements.txt + - setup.py + + install-test-deps: + desc: Install the dependencies for testing + cmds: + - pip install -r tests/requirements.txt + sources: + - tests/requirements.txt + + lint: + desc: Run linting checks + deps: + - install-pre-commit + cmds: + - pre-commit run --all-files + + test: + desc: Run all tests + deps: + - install-package + - install-test-deps + cmds: + - pytest + + test-frontend: + cmds: + - task --parallel run-frontend test + + run-frontend: + desc: Run the frontend for testing purposes + dir: streamlit_folium/frontend/ + cmds: + - npm install + - npm run start + + _run-streamlit: streamlit run examples/streamlit_app.py --server.port 8599 --server.headless true + + _run-playwright: playwright codegen localhost:8599 --target=python-pytest + + generate-tests: + desc: Generate tests for the frontend + cmds: + - task --parallel _run-streamlit _run-playwright + + generate-tests-frontend: + desc: Generate tests for the frontend + cmds: + - task --parallel run-frontend _run-streamlit _run-playwright diff --git a/examples/pages/dynamic_layer_control.py b/examples/pages/dynamic_layer_control.py index 456d7a3..6ade4db 100644 --- a/examples/pages/dynamic_layer_control.py +++ b/examples/pages/dynamic_layer_control.py @@ -49,8 +49,8 @@ "dashArray": "5, 5", } -polygon_folium1 = folium.GeoJson(data=gdf1, style_function=lambda x: style_parcels) -polygon_folium2 = folium.GeoJson(data=gdf2, style_function=lambda x: style_buildings) +polygon_folium1 = folium.GeoJson(data=gdf1, style_function=lambda _x: style_parcels) +polygon_folium2 = folium.GeoJson(data=gdf2, style_function=lambda _x: style_buildings) map = folium.Map( location=START_LOCATION, diff --git a/examples/pages/dynamic_updates.py b/examples/pages/dynamic_updates.py index 48539e1..b66d5ea 100644 --- a/examples/pages/dynamic_updates.py +++ b/examples/pages/dynamic_updates.py @@ -29,14 +29,13 @@ @st.cache_data def _get_all_state_bounds() -> dict: url = "https://raw.githubusercontent.com/PublicaMundi/MappingAPI/master/data/geojson/us-states.json" - data = requests.get(url).json() - return data + return requests.get(url).json() @st.cache_data def get_state_bounds(state: str) -> dict: data = _get_all_state_bounds() - state_entry = [f for f in data["features"] if f["properties"]["name"] == state][0] + state_entry = next(f for f in data["features"] if f["properties"]["name"] == state) return {"type": "FeatureCollection", "features": [state_entry]} @@ -109,8 +108,8 @@ def main(): st.write("## Dynamic feature group updates") - START_LOCATION = [37.7944347109497, -122.398077892527] - START_ZOOM = 17 + start_location = [37.7944347109497, -122.398077892527] + start_zoom = 17 if "feature_group" not in st.session_state: st.session_state["feature_group"] = None @@ -146,14 +145,14 @@ def main(): "dashArray": "5, 5", } - polygon_folium1 = folium.GeoJson(data=gdf1, style_function=lambda x: style_parcels) + polygon_folium1 = folium.GeoJson(data=gdf1, style_function=lambda _x: style_parcels) polygon_folium2 = folium.GeoJson( - data=gdf2, style_function=lambda x: style_buildings + data=gdf2, style_function=lambda _x: style_buildings ) map = folium.Map( - location=START_LOCATION, - zoom_start=START_ZOOM, + location=start_location, + zoom_start=start_zoom, tiles="OpenStreetMap", max_zoom=21, ) diff --git a/examples/pages/geojson_styles.py b/examples/pages/geojson_styles.py index 0d53be1..8ea832e 100644 --- a/examples/pages/geojson_styles.py +++ b/examples/pages/geojson_styles.py @@ -20,7 +20,7 @@ style_parcels = {"fillColor": "red", "fillOpacity": 0.2} -polygon_folium = folium.GeoJson(data=gdf, style_function=lambda x: style_parcels) +polygon_folium = folium.GeoJson(data=gdf, style_function=lambda _x: style_parcels) map = folium.Map( location=START_LOCATION, zoom_start=START_ZOOM, tiles="OpenStreetMap", max_zoom=21 diff --git a/examples/park_app.py b/examples/park_app.py index c1e7b82..e8e203f 100644 --- a/examples/park_app.py +++ b/examples/park_app.py @@ -33,10 +33,9 @@ class Point: def from_dict(cls, data: Dict) -> "Point": if "lat" in data: return cls(float(data["lat"]), float(data["lng"])) - elif "latitude" in data: + if "latitude" in data: return cls(float(data["latitude"]), float(data["longitude"])) - else: - raise NotImplementedError(data.keys()) + raise NotImplementedError(data.keys()) def is_close_to(self, other: "Point") -> bool: close_lat = self.lat - 0.0001 <= other.lat <= self.lat + 0.0001 diff --git a/pyproject.toml b/pyproject.toml index be8e454..8e46680 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,10 +1,33 @@ [tool.ruff] line-length = 88 -select = ["E", "F", "I001"] + +[tool.ruff.lint] +ignore = ["B008", "ISC001", "E501", "W191", "B018"] +select = [ + "B", + "E", + "F", + "W", + "I", + "N", + "C4", + "EXE", + "ISC", + "ICN", + "PIE", + "PT", + "RET", + "SIM", + "ERA", + "PLC", + "RUF", + "ARG", +] + [tool.mypy] files = ["**/*.py"] follow_imports = "silent" ignore_missing_imports = true scripts_are_modules = true -python_version = 3.9 +python_version = "3.9" diff --git a/streamlit_folium/__init__.py b/streamlit_folium/__init__.py index d851823..7d945b5 100644 --- a/streamlit_folium/__init__.py +++ b/streamlit_folium/__init__.py @@ -1,5 +1,6 @@ from __future__ import annotations +import contextlib import hashlib import os import re @@ -47,8 +48,7 @@ def generate_js_hash( standardized_js = ( re.sub(url_pattern, "", standardized_js) + str(key) + str(return_on_hover) ) - s = hashlib.sha256(standardized_js.encode()).hexdigest() - return s + return hashlib.sha256(standardized_js.encode()).hexdigest() def folium_static( @@ -90,6 +90,7 @@ def folium_static( """ ), DeprecationWarning, + stacklevel=2, ) # if Map, wrap in Figure if isinstance(fig, folium.Map): @@ -99,9 +100,7 @@ def folium_static( ) # if DualMap, get HTML representation - elif isinstance(fig, folium.plugins.DualMap) or isinstance( - fig, branca.element.Figure - ): + if isinstance(fig, (folium.plugins.DualMap, branca.element.Figure)): return components.html(fig._repr_html_(), height=height + 10, width=width) return st_folium(fig, width=width, height=height, returned_objects=[]) @@ -113,10 +112,8 @@ def _get_siblings(fig: folium.MacroElement) -> str: html = "" if len(children) > 1: for child in children[1:]: - try: + with contextlib.suppress(Exception): html += child._template.module.html() + "\n" - except Exception: - pass return html @@ -287,8 +284,8 @@ def st_folium( # handle the case where you pass in a figure rather than a map # this assumes that a map is the first child - if not (isinstance(fig, folium.Map) or isinstance(fig, folium.plugins.DualMap)): - folium_map = list(fig._children.values())[0] + if not (isinstance(fig, (folium.Map, folium.plugins.DualMap))): + folium_map = next(iter(fig._children.values())) folium_map.render() @@ -399,7 +396,7 @@ def walk(fig): css_links.extend([href for _, href in getattr(elem, "default_css", [])]) js_links.extend([src for _, src in getattr(elem, "default_js", [])]) - component_value = _component_func( + return _component_func( script=leaflet, html=html, id=m_id, @@ -418,8 +415,6 @@ def walk(fig): js_links=js_links, ) - return component_value - def _generate_leaflet_string( m: folium.MacroElement, @@ -477,15 +472,13 @@ def _generate_leaflet_string( return leaflet, mappings for idx, child in enumerate(m._children.values()): - try: + with contextlib.suppress(UndefinedError, AttributeError): leaflet += ( "\n" + _generate_leaflet_string( child, base_id=f"{base_id}_{idx}", mappings=mappings )[0] ) - except (UndefinedError, AttributeError): - pass return leaflet, mappings @@ -520,6 +513,4 @@ def generate_leaflet_string( """ leaflet, mappings = _generate_leaflet_string(m, nested=nested, base_id=base_id) - leaflet = _replace_folium_vars(leaflet, mappings) - - return leaflet + return _replace_folium_vars(leaflet, mappings) diff --git a/streamlit_folium/frontend/public/index.html b/streamlit_folium/frontend/public/index.html index 2f918cd..57eb398 100644 --- a/streamlit_folium/frontend/public/index.html +++ b/streamlit_folium/frontend/public/index.html @@ -1,5 +1,5 @@ - + Streamlit Folium Component diff --git a/tests/test_frontend.py b/tests/test_frontend.py index a5e198b..cedef74 100644 --- a/tests/test_frontend.py +++ b/tests/test_frontend.py @@ -12,22 +12,22 @@ @pytest.fixture(scope="module", autouse=True) -def before_module(): +def _before_module(): # Run the streamlit app before each module with run_streamlit(): yield -@pytest.fixture(scope="function", autouse=True) -def before_test(page: Page): +@pytest.fixture(autouse=True) +def _before_test(page: Page): page.goto(f"localhost:{PORT}") page.set_viewport_size({"width": 2000, "height": 2000}) expect.set_options(timeout=5_000) # Take screenshot of each page if there are failures for this session -@pytest.fixture(scope="function", autouse=True) -def after_test(page: Page, request): +@pytest.fixture(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) @@ -356,7 +356,6 @@ def test_dynamic_feature_group_update(page: Page): def test_layer_control_dynamic_update(page: Page): page.get_by_role("link", name="dynamic layer control").click() - # page.get_by_text("Show generated code").click() page.frame_locator('iframe[title="streamlit_folium\\.st_folium"]').get_by_text( "Parcels"