Conversation
Updated the handling of GeoJSON inputs across multiple files to utilize the new `geojson_to_gdf` function. This change improves code consistency and simplifies the process of reading GeoJSON data from both local files and URLs. Adjustments made in `common.py`, `foliumap.py`, `leafmap.py`, and `maplibregl.py` to replace direct calls to `gpd.read_file` with the new function.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughGeoJSON loading now routes HTTP and file inputs through a shared helper across multiple map entry points. Workflow matrices and one dependency entry also move to Python 3.13. ChangesHTTP GeoJSON loading via geojson_to_gdf
Python 3.13 support
Estimated code review effort: 3 (Moderate) | ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Tick the box to add this pull request to the merge queue (same as
|
There was a problem hiding this comment.
Pull request overview
This PR refactors GeoJSON ingestion across the library by routing string GeoJSON inputs (local paths and URLs) through the shared common.geojson_to_gdf() helper, reducing duplicated geopandas.read_file() usage and standardizing conversion to GeoJSON interfaces.
Changes:
- Added HTTP URL handling to
common.geojson_to_gdf()and updated callers to use it. - Replaced direct
gpd.read_file(...).__geo_interface__calls withgeojson_to_gdf(...).__geo_interface__in MapLibre and Leafmap/Folium map entry points. - Extended some MapLibre vector-editing flows to load GeoJSON from HTTP URLs via the shared helper.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| leafmap/common.py | Extends geojson_to_gdf to handle GeoJSON URLs via HTTP fetch and from_features. |
| leafmap/maplibregl.py | Routes multiple GeoJSON/vector entry points through geojson_to_gdf for URL/file handling. |
| leafmap/leafmap.py | Uses geojson_to_gdf for reading GeoJSON inputs instead of direct gpd.read_file. |
| leafmap/foliumap.py | Uses geojson_to_gdf for reading GeoJSON inputs instead of direct gpd.read_file. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Updated the Python version in multiple GitHub Actions workflows to 3.13, ensuring compatibility with the latest features and improvements. Changes made in docs.yml, installation.yml, macos.yml, ubuntu.yml, and windows.yml.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
leafmap/maplibregl.py (3)
4339-4348: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSame redundant HTTP/local branching as
add_vector.
geojson_to_gdfalready falls back togpd.read_fileinternally for non-HTTP strings, so thisif/elsecan be collapsed to a singlegeojson_to_gdf(source).__geo_interface__call, consistent with the DRY suggestion onadd_vectorabove.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@leafmap/maplibregl.py` around lines 4339 - 4348, The source-loading branch in the string-handling path is redundant and should be simplified to match the DRY approach used in add_vector. In the logic around source_name and geojson handling, remove the explicit HTTP vs local file if/else and call geojson_to_gdf(source).__geo_interface__ directly, since geojson_to_gdf already handles non-HTTP inputs internally.
9972-9980: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSame simplification applies to
edit_vector_data.Same duplication as
add_vector_editor: thehttp/elsepair is redundant now thatgeojson_to_gdfinternally falls back togpd.read_filefor non-HTTP paths.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@leafmap/maplibregl.py` around lines 9972 - 9980, The same redundant http/else branching appears in edit_vector_data as in add_vector_editor: the URL check is unnecessary because geojson_to_gdf already falls back to gpd.read_file for local paths. Update edit_vector_data to match the simplified add_vector_editor flow by keeping the parquet handling and then calling geojson_to_gdf for the remaining string filenames, using the edit_vector_data and geojson_to_gdf symbols to locate the change.
7834-7842: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHTTP branch duplicates
geojson_to_gdf's internal fallback.The
elif filename.startswith("http")/else: gpd.read_file(filename)pair can be merged now thatgeojson_to_gdfhandles both cases; only the parquet special-case needs to remain separate.♻️ Proposed simplification
if ext in [".parquet", ".pq", ".geoparquet"]: gdf = gpd.read_parquet(filename) - elif filename.startswith("http"): - gdf = geojson_to_gdf(filename) else: - gdf = gpd.read_file(filename) + gdf = geojson_to_gdf(filename)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@leafmap/maplibregl.py` around lines 7834 - 7842, The filename handling in maplibregl.py duplicates the HTTP fallback logic inside geojson_to_gdf, so simplify the branch in the file-loading block by keeping only the parquet extension special-case and routing all non-parquet inputs through geojson_to_gdf. Update the conditional around gpd.read_parquet and geojson_to_gdf so the helper function becomes the single path for HTTP and local non-parquet sources, using the existing filename check in the same load logic.leafmap/common.py (1)
3946-3958: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winStrip
encodingbefore thefrom_featuresfallback.If
gpd.read_file()fails here, this branch still passesencoding(and any other reader-only kwargs) intoGeoDataFrame.from_features, which only acceptscrsandcolumnsand will raiseTypeError. Filter those kwargs before calling it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@leafmap/common.py` around lines 3946 - 3958, The fallback in the GeoJSON handling path passes reader-only kwargs like encoding into GeoDataFrame.from_features, which only supports a limited set of arguments. In the branch inside the GeoJSON loader function where gpd.read_file falls back to from_features, strip out encoding and any other unsupported kwargs before calling gpd.GeoDataFrame.from_features, while preserving only the valid options such as crs and columns.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@leafmap/common.py`:
- Around line 3953-3954: The HTTP fetch in the in_geojson branch uses
requests.get without any timeout or response validation, which can hang or fail
silently. Update the in_geojson handling path to use the existing request flow
in this function with a timeout, check the response status before parsing, and
wrap the fetch/JSON decode in error handling so failures are surfaced clearly.
Keep the change localized to the branch that handles string URLs starting with
http and preserve the behavior for non-HTTP inputs.
In `@leafmap/maplibregl.py`:
- Around line 11113-11120: The `DateFilterWidget.__init__` source-loading branch
duplicates the same URL/file check already handled inside `geojson_to_gdf`.
Simplify the logic where `gdfs.append(gdf)` is built by removing the
`isinstance(source, str) and source.startswith("http")` conditional and calling
`geojson_to_gdf(source)` directly for non-None sources, or extract a small
shared helper used by all repeated sites in this module.
- Around line 1680-1684: The HTTP/file branching here is duplicated because
geojson_to_gdf already decides whether to fetch from a URL or read locally.
Simplify this call site in the data-loading path by replacing the manual
isinstance(data, str) and startswith("http") check with a single geojson_to_gdf
call, then use its __geo_interface__ result. Apply the same cleanup in
add_symbol and DateFilterWidget.__init__ so the logic lives only in
geojson_to_gdf.
---
Outside diff comments:
In `@leafmap/common.py`:
- Around line 3946-3958: The fallback in the GeoJSON handling path passes
reader-only kwargs like encoding into GeoDataFrame.from_features, which only
supports a limited set of arguments. In the branch inside the GeoJSON loader
function where gpd.read_file falls back to from_features, strip out encoding and
any other unsupported kwargs before calling gpd.GeoDataFrame.from_features,
while preserving only the valid options such as crs and columns.
In `@leafmap/maplibregl.py`:
- Around line 4339-4348: The source-loading branch in the string-handling path
is redundant and should be simplified to match the DRY approach used in
add_vector. In the logic around source_name and geojson handling, remove the
explicit HTTP vs local file if/else and call
geojson_to_gdf(source).__geo_interface__ directly, since geojson_to_gdf already
handles non-HTTP inputs internally.
- Around line 9972-9980: The same redundant http/else branching appears in
edit_vector_data as in add_vector_editor: the URL check is unnecessary because
geojson_to_gdf already falls back to gpd.read_file for local paths. Update
edit_vector_data to match the simplified add_vector_editor flow by keeping the
parquet handling and then calling geojson_to_gdf for the remaining string
filenames, using the edit_vector_data and geojson_to_gdf symbols to locate the
change.
- Around line 7834-7842: The filename handling in maplibregl.py duplicates the
HTTP fallback logic inside geojson_to_gdf, so simplify the branch in the
file-loading block by keeping only the parquet extension special-case and
routing all non-parquet inputs through geojson_to_gdf. Update the conditional
around gpd.read_parquet and geojson_to_gdf so the helper function becomes the
single path for HTTP and local non-parquet sources, using the existing filename
check in the same load logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2d3b48b6-c106-4c3b-b0a2-621b5d1287b8
📒 Files selected for processing (4)
leafmap/common.pyleafmap/foliumap.pyleafmap/leafmap.pyleafmap/maplibregl.py
- Remove redundant HTTP/file branching at three call sites in maplibregl.py (add_vector/add_geojson path, add_symbol, DateFilterWidget.__init__) since geojson_to_gdf() now internally branches on http URLs vs local files
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@leafmap/common.py`:
- Around line 3953-3962: The HTTP fallback in the GeoJSON loading branch is
forwarding all of `**kwargs` into `gpd.GeoDataFrame.from_features()`, but that
method only supports a limited subset and can raise a new `TypeError`. Update
the `in_geojson` HTTP fallback inside `common.py` to pass only supported
arguments (for example, `crs` and `columns`) to `GeoDataFrame.from_features()`,
and keep the original `gpd.read_file()` call unchanged.
- Line 3956: The fallback handling around the initial gpd.read_file attempt is
swallowing the original fetch/parse failure with a blind except Exception.
Update this block to capture the exception as a variable and preserve it when
entering the fallback path so the original error is not lost if the fallback
also fails; keep the change localized to the read_file/fallback logic in
common.py and ensure the eventual failure message includes the first error
details.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7ce27036-0872-426e-8e70-b01a059df4ae
📒 Files selected for processing (6)
.github/workflows/docs.yml.github/workflows/installation.yml.github/workflows/macos.yml.github/workflows/ubuntu.yml.github/workflows/windows.ymlleafmap/common.py
| elif isinstance(in_geojson, str) and in_geojson.startswith("http"): | ||
| try: | ||
| return gpd.read_file(in_geojson, encoding=encoding, **kwargs) | ||
| except Exception: | ||
| response = requests.get(in_geojson, timeout=30) | ||
| response.raise_for_status() | ||
| data = response.json() | ||
| if isinstance(data, dict) and data.get("type") == "Feature": | ||
| return gpd.GeoDataFrame.from_features([data], **kwargs) | ||
| return gpd.GeoDataFrame.from_features(data, **kwargs) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does geopandas GeoDataFrame.from_features accept arbitrary keyword arguments beyond crs and columns?
💡 Result:
No, geopandas.GeoDataFrame.from_features does not accept arbitrary keyword arguments [1][2]. The method signature is explicitly defined as classmethod GeoDataFrame.from_features(features, crs=None, columns=None), and it only supports the parameters 'features', 'crs', and 'columns' [1][2][3]. Passing additional keyword arguments will result in a TypeError.
Citations:
- 1: https://docs.geopandas.org/en/stable/docs/reference/api/geopandas.GeoDataFrame.from_features.html
- 2: https://geopandas.org/en/v1.1.3/docs/reference/api/geopandas.GeoDataFrame.from_features.html
- 3: https://geopandas.org/en/v1.1.0/docs/reference/api/geopandas.GeoDataFrame.from_features.html
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
path = Path("leafmap/common.py")
lines = path.read_text().splitlines()
start, end = 3935, 3970
for i in range(start, min(end, len(lines)) + 1):
print(f"{i:5d}: {lines[i-1]}")
PYRepository: opengeos/leafmap
Length of output: 1778
Drop unsupported kwargs in the HTTP fallback. gpd.GeoDataFrame.from_features() only accepts crs and columns, so forwarding the full **kwargs from gpd.read_file() here can turn the fallback into a new TypeError and mask the original error. Pass only the supported kwargs in this branch.
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 3956-3956: Request-controlled URL passed to requests; validate against an allowlist to prevent SSRF.
Context: requests.get(in_geojson, timeout=30)
Note: [CWE-918] Server-Side Request Forgery (SSRF).
(ssrf-requests)
🪛 Ruff (0.15.20)
[warning] 3956-3956: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@leafmap/common.py` around lines 3953 - 3962, The HTTP fallback in the GeoJSON
loading branch is forwarding all of `**kwargs` into
`gpd.GeoDataFrame.from_features()`, but that method only supports a limited
subset and can raise a new `TypeError`. Update the `in_geojson` HTTP fallback
inside `common.py` to pass only supported arguments (for example, `crs` and
`columns`) to `GeoDataFrame.from_features()`, and keep the original
`gpd.read_file()` call unchanged.
| elif isinstance(in_geojson, str) and in_geojson.startswith("http"): | ||
| try: | ||
| return gpd.read_file(in_geojson, encoding=encoding, **kwargs) | ||
| except Exception: |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Blind except Exception swallows the original fetch/parse error.
except Exception: (flagged by ruff BLE001) silently discards the failure reason from the initial gpd.read_file attempt, making it hard to diagnose why the fallback path was triggered when the fallback itself later fails.
♻️ Suggested improvement
- except Exception:
+ except Exception as e:
+ logger.debug(f"gpd.read_file failed for {in_geojson}, falling back to requests: {e}")
response = requests.get(in_geojson, timeout=30)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| except Exception: | |
| except Exception as e: | |
| logger.debug(f"gpd.read_file failed for {in_geojson}, falling back to requests: {e}") |
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 3956-3956: Request-controlled URL passed to requests; validate against an allowlist to prevent SSRF.
Context: requests.get(in_geojson, timeout=30)
Note: [CWE-918] Server-Side Request Forgery (SSRF).
(ssrf-requests)
🪛 Ruff (0.15.20)
[warning] 3956-3956: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@leafmap/common.py` at line 3956, The fallback handling around the initial
gpd.read_file attempt is swallowing the original fetch/parse failure with a
blind except Exception. Update this block to capture the exception as a variable
and preserve it when entering the fallback path so the original error is not
lost if the fallback also fails; keep the change localized to the
read_file/fallback logic in common.py and ensure the eventual failure message
includes the first error details.
Source: Linters/SAST tools
- uv was resolving the transitive pyproj (via geopandas) to 3.6.1, which has no cp313 wheel and fails to build from source (PROJ not found), breaking the 3.13 installation workflows on Linux/macOS/Windows - pyproj 3.7+ ships prebuilt cp313 wheels
- Unconditional pyproj>=3.7 conflicts with requires-python>=3.9 since pyproj 3.7+ requires Python>=3.10, making resolution unsatisfiable for 3.9-3.12 - Add an environment marker so the pin only applies on Python 3.13, where the missing cp313 wheel for pyproj 3.6.1 is the actual problem
…v run The pyproj>=3.7 pin (reverted here) could not work: leafmap declares requires-python>=3.9, and `uv run` resolves the whole version range at once (universal resolution), so pyproj is forced to 3.6.1 (the only version that supports 3.9) — which has no cp313 wheel and fails to build from source. Instead, `uv pip install .` already resolves per-interpreter and correctly installs pyproj 3.7.x (with a cp313 wheel) on Python 3.13. The failure was only the subsequent `uv run` step re-resolving universally. Run the installed .venv's python/pytest directly so no re-resolution happens. - Remove pyproj pin from requirements.txt - installation/macos/windows/ubuntu workflows: replace `uv run` in the Test import and pytest steps with the .venv interpreter
Reverting ubuntu.yml to `uv run`: switching to the per-interpreter .venv pulls in numpy>=2.1, which breaks map JSON serialization in the test suite (29 tests fail with "ndarray is not JSON serializable"). The install-only workflows (installation/macos/windows) keep the .venv fix since they only do an import check and are unaffected.
…o CI
Root-cause fixes for the CI failures and Python 3.14 support:
- sanitize_geojson (common.py): recursively convert NumPy arrays/scalars in
GeoJSON to native Python types. Newer geopandas/numpy read list-valued
feature properties (e.g. a "coordinates" property) back as ndarrays, which
are not JSON serializable and broke ipyleaflet/folium widget serialization
("Object of type ndarray is not JSON serializable") across ~30 tests.
Applied in Map.add_geojson for both leafmap.py and foliumap.py.
- fit_bounds (leafmap.py): override ipyleaflet's fit_bounds to ensure an
asyncio event loop exists first. Python 3.14 no longer provides an implicit
current loop outside a running loop, so ipyleaflet's
asyncio.ensure_future(self._fit_bounds(...)) raised RuntimeError.
- Make fiona optional (leafmap.py, foliumap.py, common.py): fiona has no
Python 3.14 wheel. It was only used to register the KML driver; the default
pyogrio engine reads KML natively, so import it lazily and fall back.
- requirements_dev.txt: fiona only on Python < 3.14 (no cp314 wheel yet).
- CI: add Python 3.14 to the ubuntu/macOS/windows/installation matrices.
ubuntu.yml now runs the installed .venv interpreter for the import check and
pytest instead of `uv run`, which re-resolves universally under
requires-python>=3.9 and pins pyproj to 3.6.1 (no cp313/cp314 wheel).
Verified locally: full test suite (80 tests) passes on Python 3.14.
The pinned uv 0.4.16 predates Python 3.14 and has no download for it
("No download found for request: cpython-3.14-..."). Bump the setup-uv
version to "latest" in the workflows that test 3.14.
docs.yml already uses 3.13; align docs-build.yml (was 3.12).
Bumping docs-build to 3.13 exposed the same universal-resolution trap: `uv run` re-resolves under requires-python>=3.9 and pins pyproj to 3.6.1, which has no cp313 wheel and fails to build (proj not found). Use the installed .venv interpreter for the import, pytest, and mkdocs steps in both docs.yml and docs-build.yml.
|
🚀 Deployed on https://6a496888129bdf2e51e07f08--opengeos.netlify.app |
Updated the handling of GeoJSON inputs across multiple files to utilize the new
geojson_to_gdffunction. This change improves code consistency and simplifies the process of reading GeoJSON data from both local files and URLs. Adjustments made incommon.py,foliumap.py,leafmap.py, andmaplibregl.pyto replace direct calls togpd.read_filewith the new function.Summary by CodeRabbit
pyproj>=3.7for Python 3.13+.