From 99de34d6f663fa12c1f9539135d492bd4cd96045 Mon Sep 17 00:00:00 2001 From: ghostiee-11 Date: Wed, 19 Aug 2026 01:58:32 +0530 Subject: [PATCH 1/9] feat: expose datashader categorical blending on hvPlot views Coloring a dense scatter by a category is a datashader job: aggregate per pixel with count_cat and blend the categories present in each one. hvPlot derives that aggregator from by= whenever datashade is set, so the whole plot is expressible declaratively, but the keywords were only reachable as loose kwargs and so invisible to anything reading the params. Promote datashade, dynspread and color_key to params. View.__init__ strips params out of kwargs, so each consumer now forwards them explicitly: get_plot puts them back for hvPlot, _get_args lets the nested explorer controls claim them, and the render-size cap reads the param rather than the kwarg it used to find there. --- lumen/tests/views/test_hvplot_datashade.py | 205 +++++++++++++++++++++ lumen/views/base.py | 34 +++- 2 files changed, 236 insertions(+), 3 deletions(-) create mode 100644 lumen/tests/views/test_hvplot_datashade.py diff --git a/lumen/tests/views/test_hvplot_datashade.py b/lumen/tests/views/test_hvplot_datashade.py new file mode 100644 index 000000000..925a7ced9 --- /dev/null +++ b/lumen/tests/views/test_hvplot_datashade.py @@ -0,0 +1,205 @@ +import pandas as pd +import pytest + +from lumen.pipeline import Pipeline +from lumen.sources.base import InMemorySource +from lumen.views import base as views_base +from lumen.views.base import hvPlotBaseView, hvPlotUIView, hvPlotView + +# ---- Fixtures ---- + +@pytest.fixture +def categorical_df(): + """Points carrying a categorical column, the shape datashader blends by.""" + return pd.DataFrame( + { + "x": [0.0, 1.0, 2.0, 3.0], + "y": [0.0, 1.0, 2.0, 3.0], + "ancestry": ["Irish", "Italian", "German", "Irish"], + } + ) + + +@pytest.fixture +def categorical_pipeline(categorical_df): + source = InMemorySource(tables={"points": categorical_df}) + return Pipeline(source=source, table="points") + + +COLOR_KEY = {"Irish": "#e41a1c", "Italian": "#377eb8", "German": "#4daf4a"} + + +class RecordingFrame(pd.DataFrame): + """DataFrame that records the kwargs its .hvplot call receives.""" + + _metadata = ["recorded"] + + @property + def _constructor(self): + return RecordingFrame + + def hvplot(self, **kwargs): + self.recorded.update(kwargs) + return object() + + +def record_hvplot_call(view, df): + """Return the kwargs hvPlot would be called with for this view.""" + frame = RecordingFrame(df) + frame.recorded = {} + view.get_plot(frame) + return frame.recorded + + +# ---- Params exist ---- + +def test_datashade_params_declared(): + """The AI agent's schema is derived from these params, so they must exist.""" + for name in ("datashade", "dynspread", "color_key"): + assert name in hvPlotBaseView.param + + +def test_datashade_params_are_not_kwargs(categorical_pipeline): + """Promoting a name to a param removes it from kwargs, which is why the + views have to forward it explicitly.""" + view = hvPlotView( + pipeline=categorical_pipeline, + kind="points", + x="x", + y="y", + by=["ancestry"], + datashade=True, + color_key=COLOR_KEY, + ) + + assert view.kwargs == {} + assert view.datashade is True + assert view.color_key == COLOR_KEY + + +# ---- hvPlotView forwards to hvPlot ---- + +def test_hvplot_view_forwards_datashade(categorical_pipeline, categorical_df): + view = hvPlotView( + pipeline=categorical_pipeline, + kind="points", + x="x", + y="y", + by=["ancestry"], + datashade=True, + dynspread=True, + color_key=COLOR_KEY, + ) + + recorded = record_hvplot_call(view, categorical_df) + + assert recorded["datashade"] is True + assert recorded["dynspread"] is True + assert recorded["color_key"] == COLOR_KEY + assert recorded["by"] == ["ancestry"] + + +def test_hvplot_view_omits_unset_datashade(categorical_pipeline, categorical_df): + """An ordinary plot must not start carrying datashader keywords.""" + view = hvPlotView(pipeline=categorical_pipeline, kind="scatter", x="x", y="y") + + recorded = record_hvplot_call(view, categorical_df) + + assert "datashade" not in recorded + assert "dynspread" not in recorded + assert "color_key" not in recorded + + +def test_hvplot_view_keeps_dict_cmap_kwarg(categorical_pipeline, categorical_df): + """Specs predating color_key passed the mapping as cmap; hvPlot still maps a + dict cmap onto color_key, so those must keep working.""" + view = hvPlotView( + pipeline=categorical_pipeline, + kind="points", + x="x", + y="y", + by=["ancestry"], + datashade=True, + cmap=COLOR_KEY, + ) + + recorded = record_hvplot_call(view, categorical_df) + + assert recorded["cmap"] == COLOR_KEY + assert recorded["datashade"] is True + + +# ---- hvPlotUIView forwards to the explorer ---- + +def test_hvplot_ui_view_forwards_datashade(categorical_pipeline): + """The explorer keeps these on nested controls rather than on + hvPlotExplorer itself, so _get_args has to look there.""" + view = hvPlotUIView( + pipeline=categorical_pipeline, + kind="points", + x="x", + y="y", + by=["ancestry"], + datashade=True, + dynspread=True, + color_key=COLOR_KEY, + ) + + _args, kwargs = view._get_args() + + assert kwargs["datashade"] is True + assert kwargs["dynspread"] is True + assert kwargs["color_key"] == COLOR_KEY + + +def test_hvplot_ui_view_builds_explorer(categorical_pipeline): + """hvPlotExplorer.__init__ raises on any keyword no control claims, so + constructing it proves the forwarded names are routable.""" + view = hvPlotUIView( + pipeline=categorical_pipeline, + kind="points", + x="x", + y="y", + by=["ancestry"], + datashade=True, + color_key=COLOR_KEY, + ) + + explorer = view.get_panel() + + assert explorer.operations.datashade is True + assert explorer.colormapping.color_key == COLOR_KEY + + +# ---- Render-size cap ---- + +def test_render_cap_exempts_datashade_param(categorical_pipeline, monkeypatch): + """The cap reads the param now; reading only kwargs would reject every + datashaded plot, which is exactly the large-frame case datashade is for.""" + monkeypatch.setattr(views_base, "MAX_RENDER_ROWS", 2) + view = hvPlotView( + pipeline=categorical_pipeline, kind="points", x="x", y="y", datashade=True + ) + over_cap = pd.DataFrame({"x": range(10), "y": range(10)}) + + view._check_render_size(over_cap) # datashade -> exempt -> must not raise + + +def test_render_cap_still_exempts_rasterize_kwarg(categorical_pipeline, monkeypatch): + """rasterize stays a plain kwarg and must keep its exemption.""" + monkeypatch.setattr(views_base, "MAX_RENDER_ROWS", 2) + view = hvPlotView( + pipeline=categorical_pipeline, kind="points", x="x", y="y", rasterize=True + ) + over_cap = pd.DataFrame({"x": range(10), "y": range(10)}) + + view._check_render_size(over_cap) + + +def test_render_cap_still_raises_without_aggregation(categorical_pipeline, monkeypatch): + monkeypatch.setattr(views_base, "MAX_RENDER_ROWS", 2) + view = hvPlotView(pipeline=categorical_pipeline, kind="points", x="x", y="y") + over_cap = pd.DataFrame({"x": range(10), "y": range(10)}) + + with pytest.raises(ValueError, match="10 rows"): + view._check_render_size(over_cap) diff --git a/lumen/views/base.py b/lumen/views/base.py index d742a1417..015bde569 100644 --- a/lumen/views/base.py +++ b/lumen/views/base.py @@ -967,6 +967,20 @@ class hvPlotBaseView(View): by = param.ListSelector(doc="The column(s) to facet the plot by.") + color_key = param.Dict(default=None, doc=""" + Mapping of the values in `by` to explicit colors, e.g. + {'Irish': '#e41a1c', 'Italian': '#377eb8'}. Only meaningful with + datashade; without it datashader picks a categorical palette.""") + + datashade = param.Boolean(default=False, doc=""" + Aggregate the data server-side with datashader and send an image + instead of one glyph per row. Combined with `by` this blends the + categories present in each pixel, rather than overplotting them.""") + + dynspread = param.Boolean(default=False, doc=""" + Grow isolated points so sparse regions stay visible after + datashading. Has no effect unless datashade is enabled.""") + groupby = param.ListSelector(doc="The column(s) to group by.") z = param.Selector(doc=""" @@ -1013,7 +1027,7 @@ def _check_render_size(self, df) -> None: n = len(df) if n <= MAX_RENDER_ROWS or self.kind in REDUCING_KINDS: return - if self.kwargs.get('rasterize') or self.kwargs.get('datashade'): + if self.datashade or self.kwargs.get('rasterize'): return raise ValueError( f"Cannot render {n:,} rows as kind={self.kind!r}: each row becomes a " @@ -1036,14 +1050,20 @@ class hvPlotUIView(hvPlotBaseView): view_type = 'hvplot_ui' def _get_args(self, explorer_cls=None, data=None): - from hvplot.ui import Geographic, hvPlotExplorer # type: ignore + from hvplot.ui import ( # type: ignore + Colormapping, Geographic, Operations, hvPlotExplorer, + ) if explorer_cls is None: explorer_cls = hvPlotExplorer if data is None: data = self.get_data() + # The explorer keeps colormapping and datashading on nested controls, so + # a param is only forwarded if one of them claims it; anything else is + # rejected by hvPlotExplorer.__init__. + controls = (explorer_cls.param, Geographic.param, Colormapping.param, Operations.param) params = { k: v for k, v in self.param.values().items() - if (k in explorer_cls.param or k in Geographic.param) + if any(k in control for control in controls) and v is not None and k != 'name' } return (data,), dict(params, **self.kwargs) @@ -1187,6 +1207,14 @@ def get_plot(self, df): processed['stream'] = self._data_stream if self.z is not None: processed['C' if self.kind == 'heatmap' else 'z'] = self.z + # Params are stripped out of kwargs by View.__init__, so anything hvPlot + # needs has to be put back explicitly. + if self.datashade: + processed['datashade'] = True + if self.dynspread: + processed['dynspread'] = True + if self.color_key is not None: + processed['color_key'] = self.color_key kind = self.kind plot_source = df From 647ff296028e50fc135efe9d832764a117142c75 Mon Sep 17 00:00:00 2001 From: ghostiee-11 Date: Wed, 19 Aug 2026 02:24:58 +0530 Subject: [PATCH 2/9] feat: let the plot agent reach for categorical datashading The agent derives its output schema from the view's params, so the datashade, dynspread and color_key params are now fields the model can fill; the prompt says when to reach for them. Above the row threshold the agent forced rasterize, which reduces each pixel to a single number and so discards the category it was asked to color by. Pick datashade instead whenever by is set, and leave the scalar path alone otherwise. --- lumen/ai/agents/hvplot.py | 9 ++- lumen/ai/prompts/hvPlotAgent/main.jinja2 | 6 ++ lumen/tests/ai/test_hvplot_datashade.py | 83 ++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 2 deletions(-) create mode 100644 lumen/tests/ai/test_hvplot_datashade.py diff --git a/lumen/ai/agents/hvplot.py b/lumen/ai/agents/hvplot.py index 4cad1d94c..83beb67b3 100644 --- a/lumen/ai/agents/hvplot.py +++ b/lumen/ai/agents/hvplot.py @@ -70,6 +70,11 @@ async def _extract_spec(self, context: TContext, spec: dict[str, Any]): spec["responsive"] = True data = await get_data(pipeline) if len(data) > 20000 and spec["kind"] in ("line", "scatter", "points"): - spec["rasterize"] = True - spec["cnorm"] = "log" + if spec.get("by"): + # rasterize reduces to a single number per pixel, which throws + # away the category; datashade blends the ones sharing a pixel. + spec["datashade"] = True + else: + spec["rasterize"] = True + spec["cnorm"] = "log" return spec diff --git a/lumen/ai/prompts/hvPlotAgent/main.jinja2 b/lumen/ai/prompts/hvPlotAgent/main.jinja2 index d8681d4dd..16788dcc4 100644 --- a/lumen/ai/prompts/hvPlotAgent/main.jinja2 +++ b/lumen/ai/prompts/hvPlotAgent/main.jinja2 @@ -6,6 +6,12 @@ no repeated columns allowed. Do not arbitrarily set `groupby` and `by` fields un histogram requested, use `y` instead of `x`. If x categorical or strings, prefer barh over bar, and use `y` for values. If column has `format: geometry`, set `kind` from its `geometry_type`: `polygons` for Polygon or MultiPolygon, `paths` for LineString or MultiLineString, otherwise `points`. + +To color a dense scatter or point cloud by a category, set `by` to that column and `datashade=True`; this +aggregates per pixel and blends the categories sharing one, instead of drawing points on top of each other. +Add `dynspread=True` when the points are sparse enough to disappear. Set `color_key` to an explicit +{category: hex} mapping only when the user names colors or asks for a specific palette; otherwise leave it +unset and a categorical palette is chosen. Do not set `datashade` for a plot with no `by`. {%- if gridded is defined and gridded %} For gridded xarray data (dimensions {{ gridded.dims | join(', ') }}, variables {{ gridded.data_vars | join(', ') }}), pick framing request calls for: diff --git a/lumen/tests/ai/test_hvplot_datashade.py b/lumen/tests/ai/test_hvplot_datashade.py new file mode 100644 index 000000000..c46c199f3 --- /dev/null +++ b/lumen/tests/ai/test_hvplot_datashade.py @@ -0,0 +1,83 @@ +from types import SimpleNamespace +from unittest.mock import patch + +import pandas as pd +import param +import pytest + +from pydantic import BaseModel + +from lumen.ai.agents.hvplot import hvPlotAgent +from lumen.ai.config import PROMPTS_DIR +from lumen.ai.translate import param_to_pydantic +from lumen.views.base import hvPlotBaseView + + +async def extract(spec, n_rows): + """Run _extract_spec against a frame of the given size.""" + data = pd.DataFrame({"x": range(n_rows), "y": range(n_rows), "c": ["a"] * n_rows}) + with patch("lumen.ai.agents.hvplot.get_data", return_value=data): + return await hvPlotAgent()._extract_spec( + {"pipeline": SimpleNamespace(table="t")}, dict(spec) + ) + + +# ---- The LLM has to be able to express the spec ---- + +def test_datashade_params_reach_the_pydantic_schema(): + """The agent derives the LLM's schema from the view's params, so a param + that param_to_pydantic cannot map is a field the model can never fill.""" + class Probe(param.Parameterized): + by = hvPlotBaseView.param.by + color_key = hvPlotBaseView.param.color_key + datashade = hvPlotBaseView.param.datashade + dynspread = hvPlotBaseView.param.dynspread + + models = param_to_pydantic(Probe, base_model=BaseModel, process_subclasses=False) + properties = models["Probe"].model_json_schema()["properties"] + + assert {"datashade", "dynspread", "color_key"} <= set(properties) + + +# ---- The large-frame default has to preserve the category ---- + +async def test_large_categorical_frame_uses_datashade(): + """rasterize reduces each pixel to one number, which discards the category.""" + spec = await extract({"kind": "points", "x": "x", "y": "y", "by": ["c"]}, 20_001) + + assert spec["datashade"] is True + assert "rasterize" not in spec + assert "cnorm" not in spec + + +async def test_large_frame_without_by_still_rasterizes(): + spec = await extract({"kind": "points", "x": "x", "y": "y"}, 20_001) + + assert spec["rasterize"] is True + assert spec["cnorm"] == "log" + assert "datashade" not in spec + + +async def test_small_categorical_frame_is_left_alone(): + """Under the threshold every point is drawn, so neither operation applies.""" + spec = await extract({"kind": "points", "x": "x", "y": "y", "by": ["c"]}, 10) + + assert "datashade" not in spec + assert "rasterize" not in spec + + +@pytest.mark.parametrize("kind", ["bar", "heatmap", "hist"]) +async def test_non_point_kinds_are_untouched(kind): + spec = await extract({"kind": kind, "x": "x", "y": "y", "by": ["c"]}, 20_001) + + assert "datashade" not in spec + assert "rasterize" not in spec + + +# ---- The prompt has to mention it, or the model never tries ---- + +def test_prompt_documents_categorical_datashading(): + prompt = (PROMPTS_DIR / "hvPlotAgent" / "main.jinja2").read_text() + + assert "datashade" in prompt + assert "color_key" in prompt From 74e4267f44a166e5059437f7a5dbf03fc439bcf5 Mon Sep 17 00:00:00 2001 From: ghostiee-11 Date: Wed, 19 Aug 2026 13:23:18 +0530 Subject: [PATCH 3/9] fix: build the plot agent's schema without walking all of Lumen Asking for one view's schema converted the entire component tree and then failed on the first param type it could not map, so hvPlotAgent could not produce a response model at all. Three things were wrong. Exclusions stopped at the class they were given for, but the generated models inherit, so an excluded name still arrived through its parent's model and, worse, the parent kept recursing into whatever that param pointed at. Subclass expansion is for callers that want a union over a taxonomy, not for describing a single view, and it reached down into Panel and Bokeh objects that have no JSON schema. Field dispatch matched the exact parameter class, so any subclass of a mapped type aborted the whole model rather than the one field. Propagate exclusions to base classes, describe the view on its own, and resolve unmapped parameter types through the MRO. --- lumen/ai/agents/hvplot.py | 5 +++ lumen/ai/translate.py | 41 +++++++++++++++------- lumen/tests/ai/test_hvplot_datashade.py | 29 ++++++++++++++++ lumen/tests/ai/test_translate.py | 45 +++++++++++++++++++++++++ 4 files changed, 108 insertions(+), 12 deletions(-) diff --git a/lumen/ai/agents/hvplot.py b/lumen/ai/agents/hvplot.py index 83beb67b3..1fe5c4d56 100644 --- a/lumen/ai/agents/hvplot.py +++ b/lumen/ai/agents/hvplot.py @@ -56,6 +56,11 @@ def _get_model(self, prompt_name: str, schema: dict[str, Any]) -> type[BaseModel extra_fields={ "chain_of_thought": (str, FieldInfo(description="Your thought process behind the plot.")), }, + # Only this one view is being described. Expanding subclasses is for + # callers that want a union over a taxonomy, and here it would walk + # every subclass of every base, down into Panel and Bokeh objects + # that have no JSON schema and nothing to do with a plot. + process_subclasses=False, ) return model[self.view_type.__name__] diff --git a/lumen/ai/translate.py b/lumen/ai/translate.py index e15425e68..fff9047d7 100644 --- a/lumen/ai/translate.py +++ b/lumen/ai/translate.py @@ -38,6 +38,7 @@ param.CalendarDate: DATE_TYPE, param.CalendarDateRange: tuple[DATE_TYPE], param.Parameter: object, + param.Path: str, param.Color: Color, param.Callable: Callable, } @@ -279,7 +280,17 @@ def parameter_to_field(parameter: param.Parameter, created_models: dict[str, typ elif parameter.name == "margin": type_ = float | tuple[float, float] | tuple[float, float, float, float] else: - raise NotImplementedError(f"Parameter {parameter.name!r} of {param_type.__name__!r} not supported") + # Every branch above dispatches on the exact class, so a Parameter + # subclass that is not listed lands here: Magnitude is a Number, + # Filename is a Path, and any future subclass is the same story. + # Resolving through the MRO keeps those usable rather than aborting the + # whole model; param.Parameter is mapped, so the walk always terminates. + type_ = next( + (PARAM_TYPE_MAPPING[base] for base in param_type.__mro__[1:] if base in PARAM_TYPE_MAPPING), + object, + ) + if parameter.default is not None and parameter.default is not PydanticUndefined: + field_kwargs["default"] = parameter.default if hasattr(parameter, "bounds") and parameter.bounds and type_ in [int, float]: try: @@ -329,6 +340,14 @@ def param_to_pydantic( if parameterized_name in created_models: return created_models + current_excluded: list[str] + if isinstance(excluded, str) and hasattr(parameterized, excluded): + current_excluded = getattr(parameterized, excluded, []) + elif isinstance(excluded, list): # If excluded is already a list + current_excluded = excluded + else: # Fallback: excluded is a string but not an attribute, or other unexpected type + current_excluded = [] + pydantic_model_bases = [] # Iterate over direct base classes of `parameterized` for param_parent_cls in parameterized.__bases__: @@ -344,9 +363,15 @@ def param_to_pydantic( param_parent_cls, base_model=base_model, created_models=created_models, - # schema, excluded, extra_fields are generally not propagated - # to parents unless explicitly needed, as they are often - # specific to the current class conversion. + # `excluded` has to reach the parent: the generated models + # inherit, so a name dropped only here still arrives via the + # parent's model. Leaving it out also made the caller's + # exclusions no protection against walking into whatever + # those params point at, which for a View is every other + # component in Lumen. + excluded=current_excluded, + # schema and extra_fields stay local: they describe this + # class's own fields. process_subclasses=process_subclasses, # PROPAGATE ) @@ -368,14 +393,6 @@ def param_to_pydantic( elif base_model not in pydantic_model_bases: # Should be redundant if list is empty pydantic_model_bases.append(base_model) - current_excluded: list[str] - if isinstance(excluded, str) and hasattr(parameterized, excluded): - current_excluded = getattr(parameterized, excluded, []) - elif isinstance(excluded, list): # If excluded is already a list - current_excluded = excluded - else: # Fallback: excluded is a string but not an attribute, or other unexpected type - current_excluded = [] - field_params = list(getattr(parameterized, "_field_params", [])) fields = {} diff --git a/lumen/tests/ai/test_hvplot_datashade.py b/lumen/tests/ai/test_hvplot_datashade.py index c46c199f3..ad710d499 100644 --- a/lumen/tests/ai/test_hvplot_datashade.py +++ b/lumen/tests/ai/test_hvplot_datashade.py @@ -81,3 +81,32 @@ def test_prompt_documents_categorical_datashading(): assert "datashade" in prompt assert "color_key" in prompt + + +# ---- The schema the LLM is actually handed ---- + +def test_get_model_builds_the_view_schema(): + """_get_model walked the whole component tree until it hit a param type it + could not map, so no hvPlot view schema could be built at all.""" + schema = { + "lon": {"type": "number"}, + "lat": {"type": "number"}, + "family": {"type": "string", "enum": ["Irish", "Italian"]}, + } + + model = hvPlotAgent()._get_model("main", schema) + properties = model.model_json_schema()["properties"] + + assert {"kind", "x", "y", "by", "datashade", "dynspread", "color_key"} <= set(properties) + + +def test_get_model_omits_excluded_names(): + """The generated models inherit, so anything excluded has to stay out of the + parent models too.""" + model = hvPlotAgent()._get_model("main", {"lon": {"type": "number"}}) + properties = model.model_json_schema()["properties"] + + excluded = {"pipeline", "source", "transforms", "download", "controls", + "field", "selection_group"} + + assert not (excluded & set(properties)) diff --git a/lumen/tests/ai/test_translate.py b/lumen/tests/ai/test_translate.py index a5a9e8b48..e1df2b891 100644 --- a/lumen/tests/ai/test_translate.py +++ b/lumen/tests/ai/test_translate.py @@ -1,3 +1,5 @@ +from typing import get_args + import param import pytest @@ -573,3 +575,46 @@ class PlotConfig(param.Parameterized): instance = PydanticPlotConfig(xlim=(1.0, 5.0), ylim=(-1.0, 1.0)) assert instance.xlim == (1.0, 5.0) assert instance.ylim == (-1.0, 1.0) + + +class _UnmappedTypes(param.Parameterized): + """Parameter subclasses that no branch of parameter_to_field names.""" + + alpha = param.Magnitude(default=1.0) + + yaml_file = param.Filename(default=None, check_exists=False) + + +def test_param_subclasses_resolve_through_the_mro(): + """Dispatch is by exact class, so an unlisted subclass used to abort the + whole model rather than the one field.""" + models = param_to_pydantic(_UnmappedTypes, base_model=BaseModel, process_subclasses=False) + fields = models["_UnmappedTypes"].model_fields + + assert fields["alpha"].annotation is float # Magnitude is a Number + # Filename is a Path, and allows None, so it arrives as Optional[str]. + assert str in get_args(fields["yaml_file"].annotation) + + +class _Parent(param.Parameterized): + + secret = param.String(default="hidden") + + kept = param.String(default="visible") + + +class _Child(_Parent): + + own = param.String(default="mine") + + +def test_excluded_names_do_not_return_via_the_parent(): + """The generated models inherit, so an exclusion that stops at the child + still arrives through the parent's model.""" + models = param_to_pydantic( + _Child, base_model=BaseModel, excluded=["secret"], process_subclasses=False + ) + properties = models["_Child"].model_json_schema()["properties"] + + assert "secret" not in properties + assert {"kept", "own"} <= set(properties) From d4ce987ddca306b458a8fc4909e4300b4c75bb65 Mon Sep 17 00:00:00 2001 From: ghostiee-11 Date: Wed, 19 Aug 2026 13:23:36 +0530 Subject: [PATCH 4/9] fix: accept a bare column name for by and groupby in a spec The constructor already coerced a single column name into a list, but spec validation runs first and the ListSelector rejected the string before that coercion could run, so a spec saying `by: family` failed while the equivalent Python call worked. Coerce in the validation hook as well, sharing one helper so the two paths cannot drift. --- lumen/tests/views/test_hvplot_datashade.py | 30 ++++++++++++++++++++++ lumen/views/base.py | 23 ++++++++++++++--- 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/lumen/tests/views/test_hvplot_datashade.py b/lumen/tests/views/test_hvplot_datashade.py index 925a7ced9..cbe295d89 100644 --- a/lumen/tests/views/test_hvplot_datashade.py +++ b/lumen/tests/views/test_hvplot_datashade.py @@ -203,3 +203,33 @@ def test_render_cap_still_raises_without_aggregation(categorical_pipeline, monke with pytest.raises(ValueError, match="10 rows"): view._check_render_size(over_cap) + + +# ---- A single column name where a list is expected ---- + +@pytest.mark.parametrize("key", ["by", "groupby"]) +def test_spec_accepts_a_bare_column_name(categorical_pipeline, key): + """__init__ coerces a string, but validate() runs on the raw spec first, so + without a matching hook a spec saying `by: family` was rejected before the + coercion written for it ever ran.""" + spec = {"type": "hvplot", "kind": "points", "x": "x", "y": "y", key: "ancestry"} + + validated = hvPlotView.validate(dict(spec)) + + assert validated[key] == ["ancestry"] + + +@pytest.mark.parametrize("key", ["by", "groupby"]) +def test_spec_leaves_a_list_alone(categorical_pipeline, key): + spec = {"type": "hvplot", "kind": "points", "x": "x", "y": "y", key: ["ancestry"]} + + assert hvPlotView.validate(dict(spec))[key] == ["ancestry"] + + +@pytest.mark.parametrize("key", ["by", "groupby"]) +def test_constructor_still_coerces_a_bare_column_name(categorical_pipeline, key): + view = hvPlotView( + pipeline=categorical_pipeline, kind="points", x="x", y="y", **{key: "ancestry"} + ) + + assert getattr(view, key) == ["ancestry"] diff --git a/lumen/views/base.py b/lumen/views/base.py index 015bde569..5a6eda8a6 100644 --- a/lumen/views/base.py +++ b/lumen/views/base.py @@ -1001,14 +1001,29 @@ def __init__(self, **params): import hvplot.dask # type: ignore # noqa: F401 except Exception: pass - if 'by' in params and isinstance(params['by'], str): - params['by'] = [params['by']] - if 'groupby' in params and isinstance(params['groupby'], str): - params['groupby'] = [params['groupby']] + for key in ('by', 'groupby'): + if key in params: + params[key] = self._as_column_list(params[key]) if params.get("geo") and params.get("kind") in (None, "scatter"): params["kind"] = "points" super().__init__(**params) + @staticmethod + def _as_column_list(value): + """Accept a bare column name wherever a list of them is expected.""" + return [value] if isinstance(value, str) else value + + @classmethod + def _validate_by(cls, value, spec, context): + # Spec validation runs before __init__, so without this a spec saying + # `by: family` is rejected by the ListSelector before the coercion + # above ever gets to see it. + return cls._as_column_list(value) + + @classmethod + def _validate_groupby(cls, value, spec, context): + return cls._as_column_list(value) + @classproperty def _valid_keys_(cls): return None From 6ba42f6d641cfaf67c31126d3d083acedf87dabe Mon Sep 17 00:00:00 2001 From: ghostiee-11 Date: Wed, 19 Aug 2026 14:32:34 +0530 Subject: [PATCH 5/9] fix: make an agent-written datashade spec actually render Three things the model does that the plot cannot survive. It repeats a column across the axis roles despite being told not to. A groupby naming x or y raises while the plot is built; one naming the same column as by is worse, because it pages every category into its own frame and the datashaded plot then renders successfully having blended nothing. Both are dropped before the spec is validated. It sets z on kinds that have no z, which hvPlot answers with a warning nobody reads. It names colors for the categories the user mentioned and leaves the rest out, which datashader rejects outright since it needs one per category. Fill the remainder from a palette of distinct hues, keeping the colors that were asked for. --- lumen/ai/agents/hvplot.py | 23 +++++++ lumen/tests/ai/test_hvplot_datashade.py | 43 +++++++++++++ lumen/tests/views/test_hvplot_datashade.py | 72 ++++++++++++++++++++++ lumen/views/base.py | 31 +++++++++- 4 files changed, 168 insertions(+), 1 deletion(-) diff --git a/lumen/ai/agents/hvplot.py b/lumen/ai/agents/hvplot.py index 1fe5c4d56..fe30d84ca 100644 --- a/lumen/ai/agents/hvplot.py +++ b/lumen/ai/agents/hvplot.py @@ -6,6 +6,7 @@ from pydantic.fields import FieldInfo from ...views import hvPlotUIView +from ...views.base import GRIDDED_KINDS from ..config import PROMPTS_DIR from ..context import TContext from ..translate import param_to_pydantic @@ -64,9 +65,31 @@ def _get_model(self, prompt_name: str, schema: dict[str, Any]) -> type[BaseModel ) return model[self.view_type.__name__] + @staticmethod + def _drop_conflicting_axes(spec: dict[str, Any]) -> None: + """Remove axis assignments that contradict each other. + + The prompt asks for x, y, by and groupby to name distinct columns, and + the model does not always oblige. A groupby repeating x or y raises + while the plot is built, and one repeating by is worse than that: it + pages each category into its own frame, so a datashaded plot renders + without complaint and blends nothing. + """ + taken = {spec.get("x"), spec.get("y"), *(spec.get("by") or [])} + groupby = [col for col in (spec.get("groupby") or []) if col not in taken] + if groupby: + spec["groupby"] = groupby + else: + spec.pop("groupby", None) + # z belongs to the gridded kinds; elsewhere hvPlot only warns it is + # unused, which is a warning nobody reads. + if spec.get("kind") not in GRIDDED_KINDS and spec.get("kind") != "heatmap": + spec.pop("z", None) + async def _extract_spec(self, context: TContext, spec: dict[str, Any]): pipeline = context["pipeline"] spec = {key: val for key, val in spec.items() if val is not None} + self._drop_conflicting_axes(spec) spec["type"] = "hvplot_ui" self.view_type.validate(spec) spec.pop("type", None) diff --git a/lumen/tests/ai/test_hvplot_datashade.py b/lumen/tests/ai/test_hvplot_datashade.py index ad710d499..2f5168948 100644 --- a/lumen/tests/ai/test_hvplot_datashade.py +++ b/lumen/tests/ai/test_hvplot_datashade.py @@ -110,3 +110,46 @@ def test_get_model_omits_excluded_names(): "field", "selection_group"} assert not (excluded & set(properties)) + + +# ---- The model does not always honour the "distinct columns" instruction ---- + +async def test_groupby_repeating_by_is_dropped(): + """A groupby equal to by pages each category into its own frame, so the + plot renders and blends nothing.""" + spec = await extract( + {"kind": "points", "x": "x", "y": "y", "by": ["c"], "groupby": ["c"]}, 20_001 + ) + + assert "groupby" not in spec + assert spec["datashade"] is True + + +async def test_groupby_repeating_the_axes_is_dropped(): + """A groupby naming x or y raises while the plot is being built.""" + spec = await extract( + {"kind": "points", "x": "x", "y": "y", "by": ["c"], "groupby": ["x", "y", "c"]}, 20_001 + ) + + assert "groupby" not in spec + + +async def test_a_distinct_groupby_survives(): + spec = await extract( + {"kind": "line", "x": "x", "y": "y", "by": ["c"], "groupby": ["other"]}, 10 + ) + + assert spec["groupby"] == ["other"] + + +async def test_z_is_dropped_for_non_gridded_kinds(): + spec = await extract({"kind": "points", "x": "x", "y": "y", "z": "c"}, 10) + + assert "z" not in spec + + +@pytest.mark.parametrize("kind", ["image", "quadmesh", "heatmap"]) +async def test_z_is_kept_for_gridded_kinds(kind): + spec = await extract({"kind": kind, "x": "x", "y": "y", "z": "c"}, 10) + + assert spec["z"] == "c" diff --git a/lumen/tests/views/test_hvplot_datashade.py b/lumen/tests/views/test_hvplot_datashade.py index cbe295d89..54569646f 100644 --- a/lumen/tests/views/test_hvplot_datashade.py +++ b/lumen/tests/views/test_hvplot_datashade.py @@ -233,3 +233,75 @@ def test_constructor_still_coerces_a_bare_column_name(categorical_pipeline, key) ) assert getattr(view, key) == ["ancestry"] + + +# ---- A colour key naming only some categories ---- + +def test_partial_color_key_is_completed(categorical_pipeline, categorical_df): + """Datashader needs a color per category, but naming the few that matter is + the natural way to ask, so the rest are filled rather than raising.""" + view = hvPlotView( + pipeline=categorical_pipeline, + kind="points", + x="x", + y="y", + by=["ancestry"], + datashade=True, + color_key={"Irish": "#e41a1c"}, + ) + + resolved = view._complete_color_key(categorical_df) + + assert set(resolved) == {"Irish", "Italian", "German"} + assert resolved["Irish"] == "#e41a1c" + assert len(set(resolved.values())) == 3 + + +def test_completed_key_never_reuses_a_chosen_color(categorical_pipeline, categorical_df): + view = hvPlotView( + pipeline=categorical_pipeline, + kind="points", + x="x", + y="y", + by=["ancestry"], + datashade=True, + color_key={"Irish": "#1f77b4"}, + ) + + resolved = view._complete_color_key(categorical_df) + + assert list(resolved.values()).count("#1f77b4") == 1 + + +def test_complete_key_is_left_alone(categorical_pipeline, categorical_df): + view = hvPlotView( + pipeline=categorical_pipeline, + kind="points", + x="x", + y="y", + by=["ancestry"], + datashade=True, + color_key=COLOR_KEY, + ) + + assert view._complete_color_key(categorical_df) == COLOR_KEY + + +def test_completion_covers_a_categorical_dtype(categorical_pipeline, categorical_df): + """count_cat keys off the dtype's categories, which can list values that + never appear in the frame.""" + framed = categorical_df.copy() + framed["ancestry"] = framed["ancestry"].astype("category") + view = hvPlotView( + pipeline=categorical_pipeline, + kind="points", + x="x", + y="y", + by=["ancestry"], + datashade=True, + color_key={"Irish": "#e41a1c"}, + ) + + resolved = view._complete_color_key(framed) + + assert set(resolved) == set(framed["ancestry"].cat.categories) diff --git a/lumen/views/base.py b/lumen/views/base.py index 5a6eda8a6..4f43cc66e 100644 --- a/lumen/views/base.py +++ b/lumen/views/base.py @@ -21,6 +21,7 @@ import param # type: ignore from bokeh.models import NumeralTickFormatter # type: ignore +from holoviews.plotting.util import process_cmap # type: ignore from hvplot import hvPlotTabular # type: ignore from panel.io.document import immediate_dispatch from panel.pane.base import PaneBase @@ -1028,6 +1029,32 @@ def _validate_groupby(cls, value, spec, context): def _valid_keys_(cls): return None + def _complete_color_key(self, df): + """Fill in a partial ``color_key`` from the categorical palette. + + Not named ``_resolve_color_key``: from_spec treats ``_resolve_`` + as a spec resolver and would call this with the raw spec value. + + Datashader needs a color for every category present, but naming the few + that matter and leaving the rest is the natural way to ask for one, so + the remainder are filled in rather than raising. + """ + if self.color_key is None or not self.by or not isinstance(df, pd.DataFrame): + return self.color_key + column = df[self.by[0]] + categories = list( + column.cat.categories if isinstance(column.dtype, pd.CategoricalDtype) + else pd.unique(column) + ) + missing = [c for c in categories if c not in self.color_key] + if not missing: + return self.color_key + # glasbey_hv carries 256 distinct hues; a Category palette repeats + # after 10 or 20 and would hand two categories the same color. + spare = [c for c in process_cmap('glasbey_hv', categorical=True) + if c not in set(self.color_key.values())] + return dict(self.color_key, **dict(zip(missing, spare, strict=False))) + def _check_render_size(self, df) -> None: """Refuse to render more per-row glyphs than a browser tab can hold. @@ -1081,6 +1108,8 @@ def _get_args(self, explorer_cls=None, data=None): if any(k in control for control in controls) and v is not None and k != 'name' } + if self.color_key is not None: + params['color_key'] = self._complete_color_key(data) return (data,), dict(params, **self.kwargs) def __panel__(self): @@ -1229,7 +1258,7 @@ def get_plot(self, df): if self.dynspread: processed['dynspread'] = True if self.color_key is not None: - processed['color_key'] = self.color_key + processed['color_key'] = self._complete_color_key(df) kind = self.kind plot_source = df From 2f184c2220a58d71a86de68fcac3bedc57949677 Mon Sep 17 00:00:00 2001 From: ghostiee-11 Date: Wed, 19 Aug 2026 14:52:41 +0530 Subject: [PATCH 6/9] fix: do not force color_key onto an explorer that has no such control _get_args forwards a param only when one of the explorer's controls claims it, but the color_key completion was written outside that gate. hvPlot gained the control after this was written, so on an older hvPlot the keyword reached hvPlotExplorer.__init__ and was rejected outright rather than falling back to the default palette. --- lumen/views/base.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lumen/views/base.py b/lumen/views/base.py index 4f43cc66e..564ef8e41 100644 --- a/lumen/views/base.py +++ b/lumen/views/base.py @@ -1108,7 +1108,11 @@ def _get_args(self, explorer_cls=None, data=None): if any(k in control for control in controls) and v is not None and k != 'name' } - if self.color_key is not None: + # Only completed once a control has claimed it above: hvPlot gained the + # color_key control after this was written, and forcing the keyword in + # regardless makes hvPlotExplorer.__init__ reject it outright on an + # older hvPlot rather than simply coloring from the default palette. + if 'color_key' in params: params['color_key'] = self._complete_color_key(data) return (data,), dict(params, **self.kwargs) From a757603d52c97e06e33c410c22ea5102492e8d33 Mon Sep 17 00:00:00 2001 From: ghostiee-11 Date: Wed, 19 Aug 2026 15:15:54 +0530 Subject: [PATCH 7/9] fix: CI, skip the explorer tests that need optional packages Two assumptions held on my machine and not in CI. The color_key forwarding test asserted the keyword is always passed on, but the view deliberately withholds it when the installed hvPlot has no such control, which is the case CI runs. And building the explorer builds an hvPlot converter, which asks for datashader straight away; datashader is not a Lumen dependency. Split the version-dependent assertion into its own test, add one that pins the withholding behaviour either way, and gate the explorer tests on datashader being present. hvPlotView still covers the same forwarding without it. --- lumen/tests/views/test_hvplot_datashade.py | 65 +++++++++++++++++++++- 1 file changed, 64 insertions(+), 1 deletion(-) diff --git a/lumen/tests/views/test_hvplot_datashade.py b/lumen/tests/views/test_hvplot_datashade.py index 54569646f..13efc4421 100644 --- a/lumen/tests/views/test_hvplot_datashade.py +++ b/lumen/tests/views/test_hvplot_datashade.py @@ -1,11 +1,34 @@ import pandas as pd import pytest +from hvplot.ui import Colormapping + from lumen.pipeline import Pipeline from lumen.sources.base import InMemorySource from lumen.views import base as views_base from lumen.views.base import hvPlotBaseView, hvPlotUIView, hvPlotView +# hvPlot gained the color_key control after this was written, and a keyword no +# control claims is rejected by hvPlotExplorer.__init__, so what the view may +# forward depends on the installed version. +EXPLORER_HAS_COLOR_KEY = "color_key" in Colormapping.param +requires_explorer_color_key = pytest.mark.skipif( + not EXPLORER_HAS_COLOR_KEY, reason="hvPlot has no color_key control" +) + +# Building the explorer builds an hvPlot converter, and a datashaded one asks +# for datashader immediately. It is not a Lumen dependency, so the tests that +# go through the explorer are optional; hvPlotView covers the same forwarding +# without it. +try: + import datashader +except ImportError: + datashader = None + +requires_datashader = pytest.mark.skipif( + datashader is None, reason="datashader is not installed" +) + # ---- Fixtures ---- @pytest.fixture @@ -131,6 +154,7 @@ def test_hvplot_view_keeps_dict_cmap_kwarg(categorical_pipeline, categorical_df) # ---- hvPlotUIView forwards to the explorer ---- +@requires_datashader def test_hvplot_ui_view_forwards_datashade(categorical_pipeline): """The explorer keeps these on nested controls rather than on hvPlotExplorer itself, so _get_args has to look there.""" @@ -149,9 +173,47 @@ def test_hvplot_ui_view_forwards_datashade(categorical_pipeline): assert kwargs["datashade"] is True assert kwargs["dynspread"] is True + + +@requires_datashader +@requires_explorer_color_key +def test_hvplot_ui_view_forwards_color_key(categorical_pipeline): + view = hvPlotUIView( + pipeline=categorical_pipeline, + kind="points", + x="x", + y="y", + by=["ancestry"], + datashade=True, + color_key=COLOR_KEY, + ) + + _args, kwargs = view._get_args() + assert kwargs["color_key"] == COLOR_KEY +@requires_datashader +def test_hvplot_ui_view_omits_color_key_without_the_control(categorical_pipeline): + """Forwarding a keyword no control claims makes hvPlotExplorer.__init__ + reject it outright, so on an hvPlot without the control the plot has to + fall back to the default palette rather than fail.""" + view = hvPlotUIView( + pipeline=categorical_pipeline, + kind="points", + x="x", + y="y", + by=["ancestry"], + datashade=True, + color_key=COLOR_KEY, + ) + + _args, kwargs = view._get_args() + + assert ("color_key" in kwargs) is EXPLORER_HAS_COLOR_KEY + + +@requires_datashader def test_hvplot_ui_view_builds_explorer(categorical_pipeline): """hvPlotExplorer.__init__ raises on any keyword no control claims, so constructing it proves the forwarded names are routable.""" @@ -168,7 +230,8 @@ def test_hvplot_ui_view_builds_explorer(categorical_pipeline): explorer = view.get_panel() assert explorer.operations.datashade is True - assert explorer.colormapping.color_key == COLOR_KEY + if EXPLORER_HAS_COLOR_KEY: + assert explorer.colormapping.color_key == COLOR_KEY # ---- Render-size cap ---- From 012d315b7d5d8e532796bd4547635f02a5260958 Mon Sep 17 00:00:00 2001 From: ghostiee-11 Date: Wed, 19 Aug 2026 15:20:21 +0530 Subject: [PATCH 8/9] chores: move the datashader guard beside the geopandas one tests/utils.py already exists to hold an optional dependency guard once rather than per file, so requires_datashader belongs there too. Also hoists a set out of a 256 iteration comprehension and a repeated spec lookup out of its condition. --- lumen/ai/agents/hvplot.py | 8 +++++--- lumen/tests/utils.py | 19 +++++++++++++++---- lumen/tests/views/test_hvplot_datashade.py | 15 ++------------- lumen/views/base.py | 4 ++-- 4 files changed, 24 insertions(+), 22 deletions(-) diff --git a/lumen/ai/agents/hvplot.py b/lumen/ai/agents/hvplot.py index fe30d84ca..acbf473fb 100644 --- a/lumen/ai/agents/hvplot.py +++ b/lumen/ai/agents/hvplot.py @@ -81,9 +81,11 @@ def _drop_conflicting_axes(spec: dict[str, Any]) -> None: spec["groupby"] = groupby else: spec.pop("groupby", None) - # z belongs to the gridded kinds; elsewhere hvPlot only warns it is - # unused, which is a warning nobody reads. - if spec.get("kind") not in GRIDDED_KINDS and spec.get("kind") != "heatmap": + # z belongs to the gridded kinds, plus heatmap, which takes the same + # column as C. Elsewhere hvPlot only warns that it is unused, which is + # a warning nobody reads. + kind = spec.get("kind") + if kind not in GRIDDED_KINDS and kind != "heatmap": spec.pop("z", None) async def _extract_spec(self, context: TContext, spec: dict[str, Any]): diff --git a/lumen/tests/utils.py b/lumen/tests/utils.py index 6443457d6..752371c7a 100644 --- a/lumen/tests/utils.py +++ b/lumen/tests/utils.py @@ -1,8 +1,8 @@ -"""Shared test helpers for optional geospatial dependencies. +"""Shared test helpers for optional dependencies. -Importing geopandas at module top would break collection where it is not -installed, so guard it once here and let tests import ``gpd``/``Polygon`` and -skip via ``requires_geopandas`` instead of repeating the guard per file. +Importing these at module top would break collection where they are not +installed, so guard them once here and let tests import the names and skip via +the ``requires_*`` markers instead of repeating the guard per file. """ import pytest @@ -18,3 +18,14 @@ requires_geopandas = pytest.mark.skipif( gpd is None, reason="geopandas is not installed" ) + +try: + import datashader +except ImportError: + datashader = None + +# Anything that builds a datashaded hvPlot needs it, including the explorer, +# whose converter asks for it as soon as the plot is constructed. +requires_datashader = pytest.mark.skipif( + datashader is None, reason="datashader is not installed" +) diff --git a/lumen/tests/views/test_hvplot_datashade.py b/lumen/tests/views/test_hvplot_datashade.py index 13efc4421..62e588d81 100644 --- a/lumen/tests/views/test_hvplot_datashade.py +++ b/lumen/tests/views/test_hvplot_datashade.py @@ -8,6 +8,8 @@ from lumen.views import base as views_base from lumen.views.base import hvPlotBaseView, hvPlotUIView, hvPlotView +from ..utils import requires_datashader + # hvPlot gained the color_key control after this was written, and a keyword no # control claims is rejected by hvPlotExplorer.__init__, so what the view may # forward depends on the installed version. @@ -16,19 +18,6 @@ not EXPLORER_HAS_COLOR_KEY, reason="hvPlot has no color_key control" ) -# Building the explorer builds an hvPlot converter, and a datashaded one asks -# for datashader immediately. It is not a Lumen dependency, so the tests that -# go through the explorer are optional; hvPlotView covers the same forwarding -# without it. -try: - import datashader -except ImportError: - datashader = None - -requires_datashader = pytest.mark.skipif( - datashader is None, reason="datashader is not installed" -) - # ---- Fixtures ---- @pytest.fixture diff --git a/lumen/views/base.py b/lumen/views/base.py index 564ef8e41..809aa9f9b 100644 --- a/lumen/views/base.py +++ b/lumen/views/base.py @@ -1051,8 +1051,8 @@ def _complete_color_key(self, df): return self.color_key # glasbey_hv carries 256 distinct hues; a Category palette repeats # after 10 or 20 and would hand two categories the same color. - spare = [c for c in process_cmap('glasbey_hv', categorical=True) - if c not in set(self.color_key.values())] + chosen = set(self.color_key.values()) + spare = [c for c in process_cmap('glasbey_hv', categorical=True) if c not in chosen] return dict(self.color_key, **dict(zip(missing, spare, strict=False))) def _check_render_size(self, df) -> None: From d404b9815e9137c87439a4ecb99994f666d744ff Mon Sep 17 00:00:00 2001 From: ghostiee-11 Date: Thu, 20 Aug 2026 23:58:34 +0530 Subject: [PATCH 9/9] feat: let a spec choose the datashader aggregator Datashading reduced each pixel to a row count with no way to ask for anything else, so shading by an average or a sum was out of reach. The reductions offered are the ones hvPlot's own explorer names. count_cat is left out: hvPlot builds it from by, and naming it here only gets as far as a bare string that never becomes a categorical reduction. Everything except count and any reduces a value column, which hvPlot takes as color; asking for one of those without it is answered from inside the datashader operation with a message that never mentions the spec, so it is caught up front instead. The agent drops such an aggregator, having no field to name the column with. --- lumen/ai/agents/hvplot.py | 6 ++- lumen/tests/ai/test_hvplot_datashade.py | 20 ++++++++ lumen/tests/views/test_hvplot_datashade.py | 59 ++++++++++++++++++++++ lumen/views/base.py | 39 ++++++++++++++ 4 files changed, 123 insertions(+), 1 deletion(-) diff --git a/lumen/ai/agents/hvplot.py b/lumen/ai/agents/hvplot.py index acbf473fb..56fddc018 100644 --- a/lumen/ai/agents/hvplot.py +++ b/lumen/ai/agents/hvplot.py @@ -6,7 +6,7 @@ from pydantic.fields import FieldInfo from ...views import hvPlotUIView -from ...views.base import GRIDDED_KINDS +from ...views.base import GRIDDED_KINDS, VALUE_AGGREGATORS from ..config import PROMPTS_DIR from ..context import TContext from ..translate import param_to_pydantic @@ -87,6 +87,10 @@ def _drop_conflicting_axes(spec: dict[str, Any]) -> None: kind = spec.get("kind") if kind not in GRIDDED_KINDS and kind != "heatmap": spec.pop("z", None) + # Reducing a value column needs one named, and the spec has no field for + # it, so an aggregator asking for that has nothing to work on. + if spec.get("aggregator") in VALUE_AGGREGATORS: + spec.pop("aggregator", None) async def _extract_spec(self, context: TContext, spec: dict[str, Any]): pipeline = context["pipeline"] diff --git a/lumen/tests/ai/test_hvplot_datashade.py b/lumen/tests/ai/test_hvplot_datashade.py index 2f5168948..0ada36560 100644 --- a/lumen/tests/ai/test_hvplot_datashade.py +++ b/lumen/tests/ai/test_hvplot_datashade.py @@ -153,3 +153,23 @@ async def test_z_is_kept_for_gridded_kinds(kind): spec = await extract({"kind": kind, "x": "x", "y": "y", "z": "c"}, 10) assert spec["z"] == "c" + + +# ---- Aggregator the spec cannot support ---- + +async def test_value_aggregator_is_dropped(): + """The spec has no field naming a value column, so mean has nothing to + reduce and hvPlot would raise from inside the datashader operation.""" + spec = await extract( + {"kind": "points", "x": "x", "y": "y", "rasterize": True, "aggregator": "mean"}, 10 + ) + + assert "aggregator" not in spec + + +async def test_row_counting_aggregator_survives(): + spec = await extract( + {"kind": "points", "x": "x", "y": "y", "rasterize": True, "aggregator": "count"}, 10 + ) + + assert spec["aggregator"] == "count" diff --git a/lumen/tests/views/test_hvplot_datashade.py b/lumen/tests/views/test_hvplot_datashade.py index 62e588d81..589bc775d 100644 --- a/lumen/tests/views/test_hvplot_datashade.py +++ b/lumen/tests/views/test_hvplot_datashade.py @@ -357,3 +357,62 @@ def test_completion_covers_a_categorical_dtype(categorical_pipeline, categorical resolved = view._complete_color_key(framed) assert set(resolved) == set(framed["ancestry"].cat.categories) + + +# ---- Aggregator ---- + +def test_aggregator_declared(): + assert "aggregator" in hvPlotBaseView.param + assert "count_cat" not in hvPlotBaseView.param.aggregator.objects + + +def test_aggregator_forwarded(categorical_pipeline, categorical_df): + view = hvPlotView( + pipeline=categorical_pipeline, kind="points", x="x", y="y", + rasterize=True, aggregator="count", + ) + + assert record_hvplot_call(view, categorical_df)["aggregator"] == "count" + + +def test_value_aggregator_needs_a_column(categorical_pipeline, categorical_df): + """hvPlot answers this from inside the datashader operation with a message + that never mentions the spec, so it is caught here instead.""" + view = hvPlotView( + pipeline=categorical_pipeline, kind="points", x="x", y="y", + rasterize=True, aggregator="mean", + ) + + with pytest.raises(ValueError, match="reduces a value column"): + record_hvplot_call(view, categorical_df) + + +def test_value_aggregator_accepts_a_column(categorical_pipeline, categorical_df): + view = hvPlotView( + pipeline=categorical_pipeline, kind="points", x="x", y="y", + rasterize=True, aggregator="mean", color="x", + ) + + assert record_hvplot_call(view, categorical_df)["aggregator"] == "mean" + + +def test_aggregator_needs_server_side_aggregation(categorical_pipeline, categorical_df): + view = hvPlotView( + pipeline=categorical_pipeline, kind="points", x="x", y="y", + aggregator="mean", color="x", + ) + + with pytest.raises(ValueError, match="aggregated server-side"): + record_hvplot_call(view, categorical_df) + + +@requires_datashader +def test_aggregator_reaches_the_explorer(categorical_pipeline): + view = hvPlotUIView( + pipeline=categorical_pipeline, kind="points", x="x", y="y", + rasterize=True, aggregator="count", + ) + + _args, kwargs = view._get_args() + + assert kwargs["aggregator"] == "count" diff --git a/lumen/views/base.py b/lumen/views/base.py index 809aa9f9b..dca6cd143 100644 --- a/lumen/views/base.py +++ b/lumen/views/base.py @@ -86,6 +86,15 @@ kind for kind in hvPlotTabular.__all__ if kind not in {"explorer", "dataset"} ] + list(GRIDDED_KINDS) +# The datashader reductions hvPlot's own explorer offers by name. count_cat is +# left out deliberately: hvPlot builds it from `by`, and naming it here only +# gets as far as a bare string that never becomes a categorical reduction. +AGGREGATORS = [None, "any", "count", "max", "mean", "min", "sum"] + +# These reduce a value column rather than counting rows, so hvPlot needs to be +# told which column via `color`; without it datashader cannot pick a dimension. +VALUE_AGGREGATORS = ("max", "mean", "min", "sum") + class View(MultiTypeComponent, Viewer): """ @@ -966,6 +975,12 @@ class hvPlotBaseView(View): y = param.Selector(doc="The column to render on the y-axis.") + aggregator = param.Selector(default=None, objects=AGGREGATORS, doc=""" + How datashader reduces the rows landing in one pixel, e.g. 'mean' to + shade by an average rather than a row count. Only meaningful with + datashade or rasterize; all but 'count' and 'any' reduce a value + column, which is named with `color`.""") + by = param.ListSelector(doc="The column(s) to facet the plot by.") color_key = param.Dict(default=None, doc=""" @@ -1055,6 +1070,26 @@ def _complete_color_key(self, df): spare = [c for c in process_cmap('glasbey_hv', categorical=True) if c not in chosen] return dict(self.color_key, **dict(zip(missing, spare, strict=False))) + def _check_aggregator(self, plot_kwargs) -> None: + """Refuse an aggregator that has nothing to reduce. + + Left to hvPlot this surfaces from inside the datashader operation as + "Could not determine dimension to apply 'aggregate' operation to", + which says nothing about the spec that caused it. + """ + if self.aggregator not in VALUE_AGGREGATORS: + return + if not (self.datashade or plot_kwargs.get('rasterize')): + raise ValueError( + f"aggregator={self.aggregator!r} only applies when the data is " + "aggregated server-side; set datashade or rasterize." + ) + if not (plot_kwargs.get('c') or plot_kwargs.get('color')): + raise ValueError( + f"aggregator={self.aggregator!r} reduces a value column, so one " + "must be named with color; use 'count' or 'any' to reduce rows." + ) + def _check_render_size(self, df) -> None: """Refuse to render more per-row glyphs than a browser tab can hold. @@ -1114,6 +1149,7 @@ def _get_args(self, explorer_cls=None, data=None): # older hvPlot rather than simply coloring from the default palette. if 'color_key' in params: params['color_key'] = self._complete_color_key(data) + self._check_aggregator(self.kwargs) return (data,), dict(params, **self.kwargs) def __panel__(self): @@ -1263,6 +1299,9 @@ def get_plot(self, df): processed['dynspread'] = True if self.color_key is not None: processed['color_key'] = self._complete_color_key(df) + if self.aggregator is not None: + self._check_aggregator(processed) + processed['aggregator'] = self.aggregator kind = self.kind plot_source = df