Skip to content

Commit cc91fec

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 holoviews/pandas/xarray/ unittest.mock imports to the top of test_hvplot_gridded.py (was inline inside test bodies). Replaced two em dashes in hvPlotAgent prompt and test docstring.
1 parent 19057e5 commit cc91fec

4 files changed

Lines changed: 16 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: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
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
7+
import xarray as xr
48

59
from lumen.pipeline import Pipeline
610
from lumen.sources.base import InMemorySource
@@ -37,7 +41,7 @@ def tabular_pipeline(tabular_df):
3741
# ---- Tests ----
3842

3943
def test_hvplot_z_param_accepted():
40-
"""z param is a valid Selector on hvPlotBaseView no AttributeError."""
44+
"""z param is a valid Selector on hvPlotBaseView (no AttributeError)."""
4145
assert "z" in hvPlotView.param
4246

4347

@@ -51,7 +55,6 @@ def test_hvplot_heatmap_from_longform_df(gridded_pipeline):
5155
5256
The public z= param is mapped internally to hvPlot's C= for heatmap.
5357
"""
54-
import holoviews as hv
5558
view = hvPlotView(
5659
pipeline=gridded_pipeline,
5760
kind="heatmap",
@@ -72,8 +75,6 @@ def test_hvplot_z_maps_to_C_only_for_heatmap(gridded_pipeline):
7275
For contourf/quadmesh/image the view pivots to xarray, so we patch the
7376
pivoted DataArray's hvplot accessor instead.
7477
"""
75-
from unittest.mock import MagicMock, patch
76-
7778
heatmap_view = hvPlotView(
7879
pipeline=gridded_pipeline, kind="heatmap", x="lon", y="lat", z="air",
7980
)
@@ -113,11 +114,10 @@ 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
121121
view = hvPlotView(
122122
pipeline=gridded_pipeline,
123123
kind="quadmesh",
@@ -131,7 +131,6 @@ def test_hvplot_quadmesh_from_longform_df(gridded_pipeline):
131131

132132
def test_hvplot_image_from_longform_df(gridded_pipeline):
133133
"""kind=image renders an Image from a long-form pandas DataFrame via pivot."""
134-
import holoviews as hv
135134
view = hvPlotView(
136135
pipeline=gridded_pipeline,
137136
kind="image",
@@ -163,7 +162,6 @@ def test_hvplot_gridded_skips_pivot_if_xarray_unavailable(gridded_pipeline):
163162
Build a view with a non-gridded kind so construction doesn't trigger the
164163
quadmesh render path, then flip the flag and call _to_gridded directly.
165164
"""
166-
import pandas as pd
167165
view = hvPlotView(
168166
pipeline=gridded_pipeline,
169167
kind="scatter",
@@ -178,7 +176,6 @@ def test_hvplot_gridded_skips_pivot_if_xarray_unavailable(gridded_pipeline):
178176

179177
def test_hvplot_gridded_passes_through_non_dataframe(gridded_pipeline):
180178
"""If the pipeline hands over an already-gridded xarray object, skip pivot."""
181-
import xarray as xr
182179
view = hvPlotView(
183180
pipeline=gridded_pipeline,
184181
kind="quadmesh",

0 commit comments

Comments
 (0)