Skip to content

Commit 43e7574

Browse files
committed
review: keep BaseViewAgent prompt agent-agnostic, move imports to top
Two review responses bundled: 1. Per review on #1824: BaseViewAgent/main.jinja2 is the base template inherited by hvPlotAgent, VegaLiteAgent, and DeckGLAgent. The previous gridded block named kind='quadmesh'/'image'/'contourf'/'heatmap' which are hvPlot-only concepts. VegaLite uses mark: rect, DeckGL uses layer types, so injecting hvPlot kind values into their prompt context is misleading. Kept the metadata (xarray source, dims, data_vars) in the base block; each agent's own prompt carries the syntax-specific guidance. Updated test to assert no hvPlot kinds leak into the base. 2. Style cleanup for lint discipline: moved MagicMock/patch/holoviews/ numpy/pandas imports to the top of test_hvplot_gridded.py (was inline inside test bodies). xarray is an optional dep, so it is guarded with pytest.importorskip inside the tests that actually use it to keep test-core collection working. Replaced two em dashes in hvPlotAgent prompt and a test docstring.
1 parent 19057e5 commit 43e7574

4 files changed

Lines changed: 20 additions & 16 deletions

File tree

lumen/ai/prompts/BaseViewAgent/main.jinja2

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,9 @@ Here is the current dataset to use:
1414
Data originates from an xarray dataset.
1515
- dimensions: {{ gridded.dims | join(', ') }}
1616
- data variables: {{ gridded.data_vars | join(', ') }}
17-
Prefer kind='quadmesh' for this data (it preserves continuous coordinate
18-
axes). Fall back to 'image' for strictly regular grids, 'contourf' for
19-
filled contour plots, or 'heatmap' if ordinal/categorical axes are wanted.
20-
Set x/y to coordinate dimensions and z to the value column.
17+
Use a 2D gridded encoding with axes on the coordinate dimensions and
18+
colour/value on the data variable, using whichever syntax this agent
19+
accepts (see agent-specific instructions below).
2120
{%- endif %}
2221

2322
{%- if "view" in memory and memory["view"].get("agent") and actor_name.startswith(memory["view"].get("agent")) %}

lumen/ai/prompts/hvPlotAgent/main.jinja2

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ the values.
88
{%- if gridded is defined and gridded %}
99

1010
For gridded xarray data:
11-
- Prefer kind='quadmesh' — it preserves continuous coordinate axes and is
11+
- Prefer kind='quadmesh'. It preserves continuous coordinate axes and is
1212
the right default for xarray-derived grids. Use 'image' only for strictly
1313
regular grids, 'contourf' for filled contours, or 'heatmap' if you
1414
specifically want ordinal/categorical axes.

lumen/tests/ai/test_gridded.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,11 @@ def test_baseview_prompt_includes_gridded_block():
107107
assert "xarray dataset" in rendered
108108
assert "lat, lon" in rendered
109109
assert "air" in rendered
110-
assert "kind='quadmesh'" in rendered
110+
# The base prompt must stay agent-agnostic: no hvPlot-specific kind
111+
# values, since VegaLite and DeckGL inherit this template too.
112+
assert "kind=" not in rendered
113+
assert "quadmesh" not in rendered
114+
assert "heatmap" not in rendered
111115

112116

113117
def test_baseview_prompt_no_gridded_block_for_tabular():

lumen/tests/views/test_hvplot_gridded.py

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
from unittest.mock import MagicMock, patch
2+
3+
import holoviews as hv
14
import numpy as np
25
import pandas as pd
36
import pytest
@@ -37,7 +40,7 @@ def tabular_pipeline(tabular_df):
3740
# ---- Tests ----
3841

3942
def test_hvplot_z_param_accepted():
40-
"""z param is a valid Selector on hvPlotBaseView no AttributeError."""
43+
"""z param is a valid Selector on hvPlotBaseView (no AttributeError)."""
4144
assert "z" in hvPlotView.param
4245

4346

@@ -51,7 +54,6 @@ def test_hvplot_heatmap_from_longform_df(gridded_pipeline):
5154
5255
The public z= param is mapped internally to hvPlot's C= for heatmap.
5356
"""
54-
import holoviews as hv
5557
view = hvPlotView(
5658
pipeline=gridded_pipeline,
5759
kind="heatmap",
@@ -72,8 +74,7 @@ def test_hvplot_z_maps_to_C_only_for_heatmap(gridded_pipeline):
7274
For contourf/quadmesh/image the view pivots to xarray, so we patch the
7375
pivoted DataArray's hvplot accessor instead.
7476
"""
75-
from unittest.mock import MagicMock, patch
76-
77+
pytest.importorskip("xarray")
7778
heatmap_view = hvPlotView(
7879
pipeline=gridded_pipeline, kind="heatmap", x="lon", y="lat", z="air",
7980
)
@@ -113,11 +114,11 @@ def test_hvplot_quadmesh_from_longform_df(gridded_pipeline):
113114
114115
Long-form pandas is the only data format XArraySQLSource produces, so the
115116
view must pivot it to a 2D array before handing to hvPlot, otherwise
116-
hvPlot raises NotImplementedError for quadmesh on pandas. Addresses
117-
@ahuang11's review: quadmesh/image are semantically more correct than
117+
hvPlot raises NotImplementedError for quadmesh on pandas. Addresses the
118+
review on #1823: quadmesh/image are semantically more correct than
118119
heatmap for continuous-coordinate gridded data.
119120
"""
120-
import holoviews as hv
121+
pytest.importorskip("xarray")
121122
view = hvPlotView(
122123
pipeline=gridded_pipeline,
123124
kind="quadmesh",
@@ -131,7 +132,7 @@ def test_hvplot_quadmesh_from_longform_df(gridded_pipeline):
131132

132133
def test_hvplot_image_from_longform_df(gridded_pipeline):
133134
"""kind=image renders an Image from a long-form pandas DataFrame via pivot."""
134-
import holoviews as hv
135+
pytest.importorskip("xarray")
135136
view = hvPlotView(
136137
pipeline=gridded_pipeline,
137138
kind="image",
@@ -145,6 +146,7 @@ def test_hvplot_image_from_longform_df(gridded_pipeline):
145146

146147
def test_hvplot_gridded_size_guard_raises(gridded_pipeline):
147148
"""Pivoting a grid larger than _GRIDDED_MAX_CELLS raises ValueError."""
149+
pytest.importorskip("xarray")
148150
view = hvPlotView(
149151
pipeline=gridded_pipeline,
150152
kind="quadmesh",
@@ -163,7 +165,6 @@ def test_hvplot_gridded_skips_pivot_if_xarray_unavailable(gridded_pipeline):
163165
Build a view with a non-gridded kind so construction doesn't trigger the
164166
quadmesh render path, then flip the flag and call _to_gridded directly.
165167
"""
166-
import pandas as pd
167168
view = hvPlotView(
168169
pipeline=gridded_pipeline,
169170
kind="scatter",
@@ -178,7 +179,7 @@ def test_hvplot_gridded_skips_pivot_if_xarray_unavailable(gridded_pipeline):
178179

179180
def test_hvplot_gridded_passes_through_non_dataframe(gridded_pipeline):
180181
"""If the pipeline hands over an already-gridded xarray object, skip pivot."""
181-
import xarray as xr
182+
xr = pytest.importorskip("xarray")
182183
view = hvPlotView(
183184
pipeline=gridded_pipeline,
184185
kind="quadmesh",

0 commit comments

Comments
 (0)