Skip to content

Support geometry columns end-to-end so GeoDataFrames render as maps#1903

Merged
ahuang11 merged 37 commits into
holoviz:mainfrom
ghostiee-11:feat/geopandas-schema-support
Jul 17, 2026
Merged

Support geometry columns end-to-end so GeoDataFrames render as maps#1903
ahuang11 merged 37 commits into
holoviz:mainfrom
ghostiee-11:feat/geopandas-schema-support

Conversation

@ghostiee-11

@ghostiee-11 ghostiee-11 commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator

Overview

Addresses #1809. Geometry columns were invisible to the schema, so a GeoDataFrame never routed to a map. This recognizes geometry in the schema, preserves the GeoDataFrame through the source and pipeline, and lets the Table, hvPlot and DeckGL views plus the map agents render it, so uploading a GeoJSON and asking for a choropleth now returns a map.

Demo

Screen.Recording.2026-07-05.at.5.55.38.PM.mov
Screen.Recording.2026-07-05.at.5.54.32.PM.mp4

DuckDB reports geometry columns as GEOMETRY/BLOB, not BINARY, so the old
detection never matched, and fetch_df() raises NotImplementedException on a
native GEOMETRY column. Detect geometry columns by duckdb type and re-select
them as WKB via ST_AsWKB before fetching, then rebuild a GeoDataFrame when
geopandas is available (falling back to WKB bytes otherwise).
Geometry columns have dtype-kind 'O' and fell into the object branch, which
ran unique() on shapely objects and produced a non-JSON-serializable enum.
Emit a compact {type: string, format: geometry, geometry_type} marker instead,
so the schema stays serializable and downstream consumers can detect spatial data.
Extract the geometry-aware fetch into a shared _fetch_df helper used by both
get() and execute(), so get_schema() no longer crashes on native GEOMETRY
columns. Preserves each caller's original date_as_object behavior.
hvPlotView.get_plot never forwarded geo and used the default kind, so a
GeoDataFrame's geometry did not render. Detect a GeoDataFrame and pick a
geometry-aware kind (polygons/paths/points), forwarding geo, so geometry
data plots as a map. Non-geo plots are unchanged. geoviews is required to
render geometry, so add it to the [geo] extra.
Now that get_dataframe_schema surfaces geometry columns as format 'geometry',
add a matching condition to the hvPlot and DeckGL agents so the coordinator
routes GeoDataFrame data to a map-capable view instead of a plain table/scatter.
A GeoDataFrame geometry column holds shapely objects that Bokeh cannot
serialize, so a Table view over geospatial data crashed the session on sync
(holoviz/panel#8663). Add a geometry_to_wkt helper and use it in the Table
view so geometry renders as WKT text; plots keep true geometry.
DeckGLView injected layer data with df.to_dict(records), which put raw shapely
geometry into the deck.gl spec and could not be serialized to the browser
(holoviz/panel#8663). Emit a GeoDataFrame as a GeoJSON FeatureCollection so a
GeoJsonLayer renders; plain frames keep the records form.
The explorer view produced by the hvPlot agent did not pick a geometry-aware
kind, so a geometry column would not render. Derive the kind (polygons/paths/
points) from the geometry type when the kind is unset, matching hvPlotView.
State what hvPlot and DeckGL each do with a geometry column (2D shaded shapes
vs basemap/3D) rather than hardcoding a preference between them, so the
coordinator routes from the user's request.
Add is_geodataframe and geometry_columns to util and reuse them across
get_dataframe_schema, geometry_to_wkt, and the view helpers, removing the
repeated inline geometry-dtype checks. Lift the stdlib json import in
DeckGLView to module level.
Add crs and a geographic flag to the geometry schema entry so the coordinator
can decide from data whether a basemap adds context: a geographic (lat/lon) CRS
suits a basemap map (DeckGL), a projected or absent one a plain 2D choropleth
(hvPlot). Conditions reference this fact rather than hardcoding a preference.
@codecov

codecov Bot commented Jul 5, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 47.50958% with 137 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.09%. Comparing base (eaf1c6d) to head (5aa4d33).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
lumen/tests/views/test_base.py 26.78% 41 Missing ⚠️
lumen/tests/sources/test_duckdb.py 39.58% 29 Missing ⚠️
lumen/util.py 55.88% 15 Missing ⚠️
lumen/tests/test_pipeline.py 33.33% 14 Missing ⚠️
lumen/tests/test_schema.py 63.15% 14 Missing ⚠️
lumen/sources/duckdb.py 50.00% 9 Missing ⚠️
lumen/views/base.py 38.46% 8 Missing ⚠️
lumen/tests/ai/test_controls/test_upload.py 61.53% 5 Missing ⚠️
lumen/tests/sources/test_optional_imports.py 86.66% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1903      +/-   ##
==========================================
- Coverage   71.22%   71.09%   -0.13%     
==========================================
  Files         198      198              
  Lines       34163    34510     +347     
==========================================
+ Hits        24332    24535     +203     
- Misses       9831     9975     +144     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

DuckDB stores geometry without a CRS, so from_wkb dropped it and every
GeoDataFrame came back crs=None. Add a geometry_crs param that _fetch_df
reapplies; it propagates to derived sources via create_sql_expr_source. Fixes holoviz#1904.
Surface the ingested GeoDataFrame's CRS in source_params so the upload path
(param.update) reapplies it after the WKB roundtrip.
Comment thread lumen/sources/duckdb.py Outdated
wrapped = f'SELECT {selected} FROM ({sql_expr})'
rel = cursor.execute(wrapped, params) if params else cursor.execute(wrapped)
df = rel.fetch_df(date_as_object=date_as_object)
if check_geopandas_available():

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would be nicer

if gpd := try_import_geopandas():
   ...

Comment thread lumen/sources/duckdb.py
Drop _is_geodata (the df.geometry.name check is redundant for a real
GeoDataFrame and raises otherwise) in favour of is_geodataframe. In hvPlotView
let hvplot infer the geometry kind by clearing a non-geometry default instead
of mapping it; keep the explicit kind only for the explorer, which defaults to
scatter and does not infer.
Comment thread lumen/util.py Outdated

def is_geodataframe(df):
"""Return True if df is a geopandas GeoDataFrame."""
gpd = try_import_geopandas()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is called speculatively, i.e. even when it isn't needed, we should not attempt to import geopandas here. Either we check if the dataframe has a geometry column or we check if "geopandas" in sys.modules.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, Makes sense, Will change this to gpd = sys.modules["geopandas"]

@philippjfr philippjfr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great but let's please make sure that we don't speculatively import geopandas if it's not needed.

Comment thread lumen/util.py Outdated

def check_geopandas_available():
"""Check if geopandas is installed."""
return try_import_geopandas() is not None

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove this function

@ahuang11
ahuang11 marked this pull request as draft July 8, 2026 20:36
Nothing calls it after the detectors switched to sys.modules-based checks;
try_import_geopandas covers the remaining import-and-use site. Addresses review.
Drop the hardcoded _geometry_kind and the explorer's use of it; the schema
already exposes geometry_type, so the hvPlotAgent prompt now tells the LLM to
set kind (polygons/paths/points) for a geometry column. Addresses review.
…-support

# Conflicts:
#	lumen/tests/ai/test_agents.py
#	lumen/tests/sources/test_optional_imports.py
#	lumen/util.py
@ghostiee-11
ghostiee-11 marked this pull request as ready for review July 8, 2026 20:58
The local was only needed for the geometry-kind check that moved to the LLM.
Comment thread lumen/util.py Outdated
Main removed it in favour of try_import_xarray; the conflict resolution
accidentally added it back. It had no callers.
Comment thread lumen/tests/views/test_base.py Outdated
Move the shapely/duckdb/DeckGLView/json imports out of the geometry tests to
the top, behind a guarded try/except that sets a requires_geo skip marker
(mirrors test_ui.py's optional-dep pattern). Addresses review.
The geometry view tests hung the macOS runner to its 15m timeout: importing
geoviews at module top pulled in matplotlib during collection, which deadlocks
the font cache under xdist. Check geoviews via importlib.util.find_spec (no
import) behind a requires_geoviews marker; hvplot imports it lazily at render.
Sort the DuckDB GROUP BY result by the group key before comparing it
against pandas' sorted groupby output in test_duckdb_transforms_cache.
DuckDB returns grouped rows in a nondeterministic order, so the
order-sensitive assert_frame_equal was flaky (it failed all reruns on
windows CI where the order differs consistently).
@ghostiee-11
ghostiee-11 requested a review from ahuang11 July 10, 2026 22:19
Comment thread lumen/tests/ai/test_agents.py Outdated
"""hvPlot and DeckGL agents advertise a geometry-column condition so the
coordinator routes GeoDataFrame data to a map-capable view."""
from lumen.ai.agents.deck_gl import DeckGLAgent
from lumen.ai.agents.hvplot import hvPlotAgent

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still see some nested imports; try git diff main...HEAD > GEOPANDAS.diff and pass that file to Claude to clean it all up.

@ghostiee-11
ghostiee-11 requested a review from ahuang11 July 17, 2026 16:45
Comment on lines +19 to +37
try:
import geopandas as gpd

from shapely.geometry import Polygon

from lumen.sources.duckdb import DuckDBSource
_GEO_DEPS = True
except ImportError:
_GEO_DEPS = False

requires_geo = pytest.mark.skipif(
not _GEO_DEPS, reason="geopandas, shapely or duckdb not installed"
)

# geoviews is checked without importing it: importing geoviews (-> matplotlib)
# at collection under xdist can deadlock the font cache on macOS. hvplot imports
# it lazily when the geometry plot is actually rendered.
requires_geoviews = pytest.mark.skipif(
importlib.util.find_spec("geoviews") is None, reason="geoviews not installed"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

duckdbsource is not part of geo deps
and also, doesn't gpd already require shapely?
are we ableto simplify to
requires_geoviews = pytest.mark.skipif(
importlib.util.find_spec("geopandas") is None, reason="geopandas not installed"
)

Comment on lines +18 to +23
try:
import geopandas as gpd

from shapely.geometry import Polygon
except Exception:
gpd = None

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can create a test util under conftest or something

Comment thread lumen/util.py
Comment on lines +523 to +539
def try_import_geopandas():
"""Import and return geopandas, or None if it is not installed."""
try:
import geopandas
return geopandas
except ImportError:
return None


def is_geodataframe(df):
"""Return True if df is a geopandas GeoDataFrame.

Uses an already-imported geopandas rather than importing it: a df cannot be
a GeoDataFrame unless geopandas is loaded, so this stays cheap on the hot
path and never speculatively imports geopandas.
"""
gpd = sys.modules.get("geopandas")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add a kwarg to try_import_geopandas(load=True) and if not load, return sys.modules.get("geopandas")

Do the same for xarray??

@ahuang11 ahuang11 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Talked to Aman; planning to fix the comments downstream since other PRs also need fixing.

@ahuang11
ahuang11 merged commit 59695a6 into holoviz:main Jul 17, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants