Conversation
|
🚀 Deployed on https://6977d3d3e8ed9078a5d42ff6--opengeos.netlify.app |
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive support for visualizing Zarr datasets in leafmap by integrating with titiler-xarray. Zarr is a cloud-optimized format for storing large N-dimensional arrays, making it ideal for geospatial and scientific data. The implementation includes functions for tile serving, metadata retrieval, and map visualization across both ipyleaflet and MapLibre backends.
Changes:
- Added five new functions for Zarr dataset interaction:
zarr_tile,zarr_info,zarr_bounds,zarr_variables, andzarr_statistics - Implemented
run_titiler_xarrayfunction to start a local titiler-xarray server for Zarr visualization - Added
add_zarrmethod to both ipyleaflet and MapLibre map backends with support for time-series data - Created comprehensive Jupyter notebook (111_zarr.ipynb) demonstrating Zarr visualization capabilities
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| mkdocs.yml | Adds reference to new Zarr visualization notebook (111_zarr.ipynb) |
| leafmap/stac.py | Implements core Zarr support functions including tile serving, metadata retrieval, and local server management |
| leafmap/maplibregl.py | Adds add_zarr method and imports Zarr utility functions for MapLibre backend |
| leafmap/leafmap.py | Adds add_zarr method for ipyleaflet backend with appropriate coordinate transformations |
| docs/notebooks/111_zarr.ipynb | Provides comprehensive tutorial on Zarr visualization with examples and best practices |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| app.add_middleware( | ||
| CORSMiddleware, | ||
| allow_origins=["*"], | ||
| allow_credentials=True, | ||
| allow_methods=["*"], | ||
| allow_headers=["*"], | ||
| ) |
There was a problem hiding this comment.
The CORS middleware is configured to allow all origins with allow_origins=["*"]. While this is convenient for local development and notebooks, it could be a security concern if users deploy this server in production. Consider adding a comment warning users about this configuration and suggesting they restrict origins in production environments, or provide a parameter to control CORS settings.
| if time_index is not None and "sel" not in kwargs: | ||
| kwargs["sel"] = f"time={time_index}" |
There was a problem hiding this comment.
When time_index is provided along with a custom sel parameter in kwargs, the time_index will be silently ignored due to the condition "sel" not in kwargs. This could lead to confusion for users who expect both parameters to work together. Consider either documenting this behavior more explicitly in the docstring, or merging the time selection with any existing sel parameter, or raising a warning when both are provided.
| except Exception as e: | ||
| proc.kill() | ||
| raise RuntimeError( | ||
| f"Failed to start titiler-xarray server: {e}\n" | ||
| "Make sure titiler.xarray is installed: pip install 'titiler.xarray[full]'" | ||
| ) |
There was a problem hiding this comment.
When the server fails to start and an exception is raised (lines 3156-3159), the temporary file created at line 3108 is not cleaned up. This could lead to accumulation of temporary files over multiple failed startup attempts. Consider cleaning up the temporary file before raising the RuntimeError, or using a try-finally block to ensure cleanup happens in all cases.
| titiler_endpoint (str, optional): TiTiler endpoint that supports xarray/Zarr. | ||
| Defaults to "https://giswqs-titiler-endpoint.hf.space". |
There was a problem hiding this comment.
The docstring states that the default titiler_endpoint is "https://giswqs-titiler-endpoint.hf.space", but this is inconsistent with the actual behavior. The function first checks for TITILER_XARRAY_ENDPOINT environment variable, and if that's not set, it calls check_titiler_endpoint which will use the default non-xarray endpoint. The docstring should clarify that the endpoint needs titiler-xarray support and mention checking the environment variable first.
| titiler_endpoint (str, optional): TiTiler endpoint that supports xarray/Zarr. | ||
| Defaults to "https://giswqs-titiler-endpoint.hf.space". |
There was a problem hiding this comment.
The docstring states that the default titiler_endpoint is "https://giswqs-titiler-endpoint.hf.space", but this is inconsistent with the actual behavior. The function directly calls check_titiler_endpoint(titiler_endpoint) without first checking for TITILER_XARRAY_ENDPOINT like other zarr functions do. The docstring should clarify that the endpoint needs titiler-xarray support.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
for more information, see https://pre-commit.ci
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Add support for visualizing zarr data * Update leafmap/stac.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update leafmap/stac.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update leafmap/maplibregl.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update leafmap/stac.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Update leafmap/leafmap.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Fix #1185