diff --git a/CHANGELOG.md b/CHANGELOG.md index 193e8a9..a73e8cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,8 +5,20 @@ while the major version is 0, minor bumps may include breaking changes and say s ## Unreleased +### Added + +- `sort_by` on a categorical bar ranks it by something other than its first metric, largest first, on every supported release. `"total"` ranks stacked or grouped bars by their sum. Before, Superset drew several series in name order: two stacked metrics on 4.1.4 and 5.0.0, and a grouped bar on every release. A metric (`"sort_by": "SUM(revenue)"`) ranks by its value and orders the query too, so a `row_limit` keeps the top bars by it; it need not be drawn, so two charts can share one order. `advise` warns when a `row_limit` cuts a bar ranked by `"total"`, since the query is still ordered by the first metric. +- A threshold on a horizontal bar: `annotations` there now say what Superset draws, a line standing upright at its value (a 1.0x threshold, a 4-hour limit), seen on every supported release, and the live CI fixture builds one. +- Heatmap axes: `x_label_every` and `y_label_every` label every Nth column or row, counted from the first, so an hour axis with `"x_label_every": 6` reads 0, 6, 12, 18 instead of Superset's uneven automatic spacing. `left_margin` leaves room, in px, left of the row labels. Every supported release takes all three; `decompile` reads them back and `plan` reports a change made in the UI. + +### Changed + +- Documented: no supported release gives one line of a mixed chart its own width or dash. The plugin has no such control and its transform passes none, and 6.1's ECharts Options replace the series list whole. Set a ghost line apart by colour with `dashboard.label_colors`, or draw a fixed level as an annotation. +- Design brain 12: `advise --fix` fills `left_margin: 16` on every heatmap (`default.heatmap-label-room`). On Superset 6.1.0 a heatmap's longest row label lost its first letters at the card's edge, and a theme can't reach the heatmap there. Write `left_margin` yourself, or ignore the rule for a chart, to keep your own value. + ### Fixed +- `decompile` names a bar ranked by its series' minimum, maximum or average, or ranked smallest first; before, apply quietly put the largest-first ranking back. - `decompile` (and so `adopt`) names more chart settings it can't carry when they differ from Superset's defaults: big-number font sizes and a hidden trendline, a smooth or step line, horizontal bars on a time axis, a pie's radius and hidden labels, a funnel's labels, tooltip and percentage calculation, treemap labels, a heatmap's axis sort, legend, margins, label intervals and value bounds, a histogram's normalize setting, and any chart's currency format. Before, they were dropped without a note. - `standards apply` no longer adds a header or footer row that the body already holds at its edge (the last rows for a footer, the first for a header). A dashboard built in the UI has no header or footer, so `decompile` and `adopt` read its legal line as a body row, and the standard's footer was then added below it, showing it twice. `--claim` moves such a row into the header or footer and records it; `explain` and the apply summary name it. diff --git a/chartwright/compiler.py b/chartwright/compiler.py index 5d8f634..3bcb427 100644 --- a/chartwright/compiler.py +++ b/chartwright/compiler.py @@ -301,6 +301,36 @@ def metric(m: str): "hasCustomLabel": False, } p["order_desc"] = chart.category_sort == "desc" + elif chart.sort_by: + # Largest first, as the default ranking: on top of the bottom-up horizontal axis. + ascending = chart.orientation == "horizontal" + sort_metric = chart.sort_metric() + if sort_metric is None: + # "total": the plugin sorts the categories by the sum of their series + # (SortSeriesType.Sum, utils/series.ts sortRows: 4.1.4 :200-208, 6.1.0 + # :564-572), reading x_axis_sort from 6.0.0 (Timeseries/transformProps.ts + # 6.1.0 :335-336) and x_axis_sort_series at 4.1.4 and 5.0.0 (:243-246, + # :246-249). "sum" is no label, so sortOperator.ts adds no sort of its own. + p["x_axis_sort"] = "sum" + p["x_axis_sort_series"] = "sum" + p["x_axis_sort_series_ascending"] = ascending + else: + # A metric: sortOperator.ts sorts the rows by it (no groupby, all three + # releases). As the "Sort query by" metric it also orders the query, so a + # row limit keeps the top bars by it (normalizeOrderBy, descending), and + # one that is not drawn is queried but never drawn: extractExtraMetrics.ts:35 + # adds it because its label is x_axis_sort, and extractSeries skips it. + payload = metric(sort_metric) + p["x_axis_sort"] = payload["label"] if isinstance(payload, dict) else payload + p["timeseries_limit_metric"] = payload + if chart.several_series(): + # Several series at 4.1.4 and 5.0.0: the plugin re-sorts the rows by + # x_axis_sort_series, whose panel default is the category name, unless + # it is unset (isDefined, utils/series.ts extractSeries, 4.1.4 + # :296-307, 5.0.0 :302-313). Null keeps sortOperator's order there; + # 6.0.0 dropped the control. + p["x_axis_sort_series"] = None + p["x_axis_sort_asc"] = ascending else: # Rankings read sorted by their measure, not by label order. The # horizontal axis renders bottom-up, so ascending puts the largest @@ -461,6 +491,18 @@ def metric(m: str): p["show_percentage"] = False # the control's default is true if chart.number_format: p["y_axis_format"] = chart.number_format # the cell values' format + # xscale_interval / yscale_interval: transformProps hands interval N - 1 to the + # category axis, so N labels every Nth value from the first (Heatmap/transformProps.ts + # 4.1.4 :231,238, 5.0.0 :234,241, 6.1.0 :436,445). left_margin is the grid's left + # edge, labels inside it (containLabel, :166-169, :168-171, :354-357); 'auto' is 0. + # On 6.1.0 Heatmap.tsx:25 renders without vizType, so a theme's per-type + # overrides never reach it and these are the only way to set the axes. + if chart.x_label_every is not None: + p["xscale_interval"] = chart.x_label_every + if chart.y_label_every is not None: + p["yscale_interval"] = chart.y_label_every + if chart.left_margin is not None: + p["left_margin"] = chart.left_margin elif t == "histogram": p["column"] = chart.column p["bins"] = chart.bins diff --git a/chartwright/decompile.py b/chartwright/decompile.py index 444edcd..6f321ba 100644 --- a/chartwright/decompile.py +++ b/chartwright/decompile.py @@ -161,6 +161,18 @@ def _x_labels_to_spec(p: dict, out: dict, losses: list, name: str) -> None: _COLUMN_CONFIG_SPEC = {v: k for k, v in COLUMN_CONFIG_KEYS.items()} # d3NumberFormat -> number_formats +# SortSeriesType (superset-ui-chart-controls types.ts, 4.1.4 :503, 6.1.0 :549): what a bar +# with several series sorts its categories by. +_SERIES_SORTS = ("name", "sum", "min", "max", "avg") + + +def _stored_label(m) -> str | None: + """A stored metric's label, as getMetricLabel reads it: a saved metric is its name.""" + if isinstance(m, str): + return m + return m.get("label") if isinstance(m, dict) else None + + def _number(v) -> float | int | None: """A stored bound or size as a number; None for null, "" or junk.""" if isinstance(v, bool) or v in (None, ""): @@ -655,12 +667,37 @@ def keep_row_limit() -> None: # series: x_axis_sort from 6.0.0, x_axis_sort_series at 4.1.4 and 5.0.0). several = bool(p.get("groupby")) or len(ms) > 1 sort, stored_asc = p.get("x_axis_sort"), p.get("x_axis_sort_asc") - if several and sort in (None, "") and p.get("x_axis_sort_series") == "name": - sort, stored_asc = "name", p.get("x_axis_sort_series_ascending", True) + if several and sort in (None, "") and p.get("x_axis_sort_series") in _SERIES_SORTS: + sort = p["x_axis_sort_series"] + stored_asc = p.get("x_axis_sort_series_ascending", True) if sort == x or (several and sort == "name"): # In reading order (the horizontal axis runs bottom-up). ascending = bool(stored_asc) != (p.get("orientation") == "horizontal") out["category_sort"] = "asc" if ascending else "desc" + elif sort not in (None, ""): + # A ranking by a measure, largest first (on top of a horizontal bar): the + # series' sum ("total"), or a metric, which compile also makes the query's + # "Sort query by" (timeseries_limit_metric), drawn or not. + ranked = p.get("timeseries_limit_metric") + first = metric_label(ms[0]) + if several and sort == "sum": + out["sort_by"] = "total" + elif several and sort in _SERIES_SORTS: + losses.append(Loss(name, f"bars ranked by the series' {sort!r} value not " + "preserved (the first metric on re-apply)")) + elif ranked and _stored_label(ranked) == sort and not p.get("groupby"): + m = metric_one(ranked) + if m: + out["sort_by"] = m + elif sort != first and not p.get("groupby"): + m = next((m for m in ms if metric_label(m) == sort), None) + if m: + out["sort_by"] = m + largest_first = p.get("orientation") == "horizontal" + if stored_asc is not None and bool(stored_asc) != largest_first and ( + "sort_by" in out or sort == first): + losses.append(Loss(name, "bars ranked smallest first not preserved " + "(largest first on re-apply)")) if p.get("y_axis_format") not in (None, "SMART_NUMBER"): out["number_format"] = p["y_axis_format"] gb = _groupby_one(p, losses, name) @@ -832,6 +869,20 @@ def keep_row_limit() -> None: out["number_format"] = p["y_axis_format"] if p.get("show_legend") is False: out["show_legend"] = False + # Superset's own values (-1 = automatic spacing, 'auto' = no margin) map to nothing. + for key, spec_field in (("xscale_interval", "x_label_every"), + ("yscale_interval", "y_label_every")): + every = _number(p.get(key)) + if isinstance(every, int) and 1 <= every <= 50: + out[spec_field] = every + elif every not in (None, -1): + losses.append(Loss(name, f"heatmap {key} {p[key]!r} not preserved (automatic on re-apply)")) + margin = _number(p.get("left_margin")) + if isinstance(margin, int) and 0 <= margin <= 200: + out["left_margin"] = margin + elif p.get("left_margin") not in (None, "", "auto"): + losses.append(Loss(name, f"heatmap left_margin {p['left_margin']!r} not preserved " + "(none on re-apply)")) keep_row_limit() elif spec_type == "histogram": col = p.get("column") @@ -958,6 +1009,8 @@ def keep_row_limit() -> None: mapped_here = mapped_here | {"x_axis_title", "y_axis_title"} if spec_type == "pie": mapped_here = mapped_here | {"show_total"} + if spec_type == "heatmap": + mapped_here = mapped_here | {"xscale_interval", "yscale_interval", "left_margin"} if spec_type in _AXIS_TYPES: _x_labels_to_spec(p, out, losses, name) mapped_here = mapped_here | _axis_to_spec(p, out, losses, name, spec_type) diff --git a/chartwright/design/brief.py b/chartwright/design/brief.py index e157c6c..c5dd418 100644 --- a/chartwright/design/brief.py +++ b/chartwright/design/brief.py @@ -89,8 +89,8 @@ def render_brief(audience: str, overlay: Overlay | None = None) -> str: "## Design defaults: `advise --fix` fills these, so leave them unset unless the user asks", "", "- x_label_format, compare_suffix, number_format of COUNT metrics, and on tables cell_bars,", - " page_length and search_box; show_legend on a single named series; show_value on few bars.", - " Set one yourself only when the user asks for it, and say so in your reply.", + " page_length and search_box; show_legend on a single named series; show_value on few bars;", + " a heatmap's left_margin. Set one yourself only when the user asks for it, and say so in your reply.", "- Run `chartwright advise --fix` (MCP: fix_spec) before you build. It writes each value", " into the spec file and records it in design.filled; its `fixed` entries say kind \"fill\" and why.", "- Edit the spec it wrote; never regenerate the spec from your own copy, or the fills are lost.", diff --git a/chartwright/design/defaults.py b/chartwright/design/defaults.py index dc86af2..3fd79f8 100644 --- a/chartwright/design/defaults.py +++ b/chartwright/design/defaults.py @@ -135,7 +135,7 @@ def _findings(ctx: RuleContext, fill: Fill): def _fill(rule_id: str, field: str, types, doc: str, *, superset, superset_text: str, - override: str): + override: str, since: str = "5"): def deco(decide): f = Fill(rule_id, field, frozenset(types), decide, superset, superset_text, override) FILLS[rule_id] = f @@ -143,7 +143,7 @@ def deco(decide): def fn(ctx: RuleContext): yield from _findings(ctx, f) - rule(rule_id, "info", doc, fixable=True, since="5")(fn) + rule(rule_id, "info", doc, fixable=True, since=since)(fn) return decide return deco @@ -394,3 +394,25 @@ def _value_labels(ctx: RuleContext, c): if w < min_w: return None, f"{w}/12 wide, narrower than {min_w}/12" return True, f"at most {n} bars at {w}/12 wide: each value reads without the axis" + + +# -- heatmaps ----------------------------------------------------------------------- + +# A heatmap's y labels sit inside its grid (containLabel), whose left edge is the card's +# edge while left_margin is Superset's 'auto' (Heatmap/transformProps.ts 6.1.0 :354-357). +# Seen on 6.1.0: the longest label is drawn wider than the room the grid gave it and +# loses its first letters there ('rucks and Buses', a 95 px label; 'ustralian Gift +# Network, Co', 150 px). 8 px cleared the first and 16 px both; 4.1.4 and 5.0.0 drew +# them whole. Its own spec field is the only way: on 6.1.0 Heatmap.tsx:25 renders +# without vizType, so a theme's per-chart-type overrides never reach it. +HEATMAP_LABEL_ROOM = 16 + + +@_fill("default.heatmap-label-room", "left_margin", {"heatmap"}, + "a heatmap keeps 16 px left of its y labels, where Superset 6.1.0 cuts off the " + "longest one's first letters", + superset=None, superset_text="no margin (Superset's 'auto')", + override="write left_margin yourself (0 for none)", since="12") +def _heatmap_label_room(ctx: RuleContext, c): + return HEATMAP_LABEL_ROOM, ("Superset 6.1.0 cuts the first letters off the longest y " + "label at the card's edge; 16 px keeps labels up to ~300 px whole") diff --git a/chartwright/design/guidelines/chart-choice.md b/chartwright/design/guidelines/chart-choice.md index 92cb934..cd21695 100644 --- a/chartwright/design/guidelines/chart-choice.md +++ b/chartwright/design/guidelines/chart-choice.md @@ -22,11 +22,18 @@ scrollbar; smoke warns at apply time when they do. - Sort order is per family: bars sort by their first metric (right for rankings, wrong for ordinals like weekday/month) unless `category_sort` - orders them by label; heatmap and pivot categories sort alphabetically + orders them by label. Stacked or grouped bars need `sort_by: "total"` to + rank: Superset puts several series in name order (4.1.4/5.0.0 always, a + groupby everywhere). `sort_by` a metric ranks by one the chart needn't draw, + so two charts share an order. Heatmap and pivot categories sort alphabetically (a pivot can sort by value: `row_order`). For ordinal dimensions, chart an order-encoded label column (labels prefixed with a sort index: '1-Mon') if the dataset has one, and give a bar `category_sort: "asc"`. An ordered axis such as hours stays vertical: add `x_label_every` so every label shows. + A heatmap's `x_label_every`/`y_label_every` is a step: 6 on hours reads 0, 6, 12, 18. +- No release gives a mixed chart's line its own width or dash: set a ghost line + apart by colour (`dashboard.label_colors`, a pale grey on its series label); + a fixed level is an annotation, which takes `style` and `width`. - A paged table (`page_length`) needs room for one page and its pager, not for every row; a timeseries with many groups keeps the top few with `series_limit`. diff --git a/chartwright/design/model.py b/chartwright/design/model.py index ad50749..6ab21b5 100644 --- a/chartwright/design/model.py +++ b/chartwright/design/model.py @@ -26,7 +26,12 @@ # "7" = fills keep a null record when the author edits one (an edit later deleted stays # deleted), and default.stale-record: a fixable finding for a renamed or removed chart's # design.filled entry, which now validates. Both change what `--fix` writes. -DESIGN_BRAIN_VERSION = "7" +# "12" = default.heatmap-label-room, a fill (left_margin on every heatmap, which 6.1.0 +# otherwise draws with its longest y label cut off); it changes what `--fix` writes. +# With it, a bar's sort_by: data.top-n-sort warns on a row_limit under sort_by "total" +# (the query is still ordered by the first metric), and chart.ordinal-order leaves a +# bar ranked by sort_by alone. ("8" to "11" are other branches'.) +DESIGN_BRAIN_VERSION = "12" KPI_TYPES = {"big_number_total", "big_number_trend"} TIMESERIES_TYPES = {"timeseries_line", "timeseries_bar", "timeseries_area", "timeseries_scatter"} diff --git a/chartwright/design/rules.py b/chartwright/design/rules.py index a9618a4..10eb8bd 100644 --- a/chartwright/design/rules.py +++ b/chartwright/design/rules.py @@ -766,6 +766,8 @@ def dims(c): for c in ctx.spec.charts: hits = [d for d in dims(c) if d and _ORDINAL_RE.search(d)] + if c.type == "bar" and c.sort_by: + continue # ranked by a measure the author chose: the order is deliberate if hits and c.type == "bar" and not c.category_sort: # A bar sorts by its first metric unless category_sort is set, so an # order-encoded label alone changes nothing. @@ -863,6 +865,21 @@ def top_n_sort(ctx: RuleContext): "the query by the series ranking, so the axis shows gaps; drop row_limit, " "or drop series_limit so the query is ordered by the category", ) + elif (c.type == "bar" and c.sort_by == "total" and c.row_limit is not None + and c.row_limit < DEFAULT_ROW_LIMIT["bar"]): + # "total" is the plugin's sort of the rows it gets (SortSeriesType.Sum); the + # query is still ordered by the first metric (normalizeOrderBy.ts), so the + # row limit cuts by that metric before the bars are ranked by their sum. + remedy = ("write the total as a metric (e.g. \"SQL(SUM(a) + SUM(b)) AS Total\") " + "and sort_by it, which orders the query too" if not c.groupby else + "with a groupby the limit also counts each bar's segments, not bars: " + "drop row_limit, or filter the categories") + yield Finding( + "data.top-n-sort", "warn", c.name, ctx.where(c.name), + f"row_limit {c.row_limit} with sort_by \"total\" keeps the rows with the " + f"largest {c.metrics[0]} and then ranks them by their total, so a bar with " + f"a large total but a small {c.metrics[0]} can be left out; {remedy}", + ) _RANGE_DAYS = {"day": 1, "week": 7, "month": 30.4, "quarter": 91, "year": 365} diff --git a/chartwright/resolver.py b/chartwright/resolver.py index 05856f2..b6dbdd3 100644 --- a/chartwright/resolver.py +++ b/chartwright/resolver.py @@ -317,6 +317,8 @@ def _check_chart_fields(chart, ds: ResolvedDataset, res: Resolution) -> None: elif t == "bar": for m in chart.metrics: _check_metric(m, chart.name, ds, res) + if chart.sort_metric() and chart.sort_metric() not in chart.metrics: + _check_metric(chart.sort_metric(), chart.name, ds, res) _check_column(chart.x_column, chart.name, ds, res, "x_column") if chart.groupby: _check_column(chart.groupby, chart.name, ds, res, "groupby") diff --git a/chartwright/smoke.py b/chartwright/smoke.py index 2a9cce8..47ccefe 100644 --- a/chartwright/smoke.py +++ b/chartwright/smoke.py @@ -144,6 +144,9 @@ def metric(m: str): elif t == "bar": q["metrics"] = [metric(m) for m in chart.metrics] q["columns"] = [chart.x_column] + ([chart.groupby] if chart.groupby else []) + if chart.sort_metric() and chart.sort_metric() not in chart.metrics: + # The chart queries a sort metric it doesn't draw (extractExtraMetrics.ts). + q["metrics"].append(metric(chart.sort_metric())) elif t == "pie": q["metrics"] = [metric(chart.metric)] q["columns"] = [chart.groupby] diff --git a/chartwright/spec.py b/chartwright/spec.py index 9912ad3..c7d6f7a 100644 --- a/chartwright/spec.py +++ b/chartwright/spec.py @@ -326,12 +326,17 @@ class _ColorSchemeMixin(BaseModel): class Annotation(BaseModel): """A FORMULA annotation layer: a line drawn from a formula in x, e.g. a goal line at y = 80. Superset draws it as a series named ``name`` (in the legend - and tooltip); formula layers have no on-chart label in any release.""" + and tooltip); formula layers have no on-chart label in any release. On a + horizontal bar the value axis runs across, so the line stands upright at its + value (transformFormulaAnnotation swaps the point to [y, x] there).""" model_config = ConfigDict(extra="forbid") name: str = Field(min_length=1, description="Series name in the legend and tooltip, e.g. \"Goal\"") - value: float | None = Field(default=None, description="A flat line at this y value, e.g. 80") + value: float | None = Field( + default=None, + description="A line at this value on the value axis, e.g. 80: flat across most charts, " + "upright across a horizontal bar (a 1.0x threshold, a 4-hour limit)") formula: str | None = Field( default=None, min_length=1, description="Instead of value: a formula in x, e.g. \"2*x + 10\" (x is the " @@ -473,7 +478,8 @@ class _AxisChart(_ChartBase): annotations: list[Annotation] = Field( default_factory=list, description="Formula lines over the chart, e.g. a goal: [{\"name\": \"Goal\", \"value\": 80, " - "\"style\": \"dashed\"}] (Superset's FORMULA annotation layers)", + "\"style\": \"dashed\"}] (Superset's FORMULA annotation layers); on a " + "horizontal bar a value stands upright across the bars", ) @model_validator(mode="after") @@ -636,14 +642,57 @@ class BarChart(_SeriesDisplay, _Legend, _AxisChart, _ColorSchemeMixin): "stays ordered by the series ranking (Superset has one \"Sort query by\" " "control for both), so there a row_limit keeps the largest values", ) + sort_by: str | None = Field( + default=None, min_length=1, + description="Rank the bars by something other than the first metric, largest first " + "(on the left, or on top of a horizontal bar). \"total\" ranks stacked or " + "grouped bars by their sum; a metric, written like any metric, ranks them " + "by its value and orders the query by it, so a row_limit keeps the top " + "bars by it. The metric need not be drawn: two charts sorted by the same " + "one share an order. A metric needs no groupby; with \"total\" a row_limit " + "keeps the rows with the largest first metric, so for a top N by total " + "write the total as a metric, e.g. \"SQL(SUM(a) + SUM(b)) AS Total\"", + ) + + TOTAL: ClassVar[str] = "total" @model_validator(mode="after") def _display(self) -> "BarChart": self._check_legend() self._check_y_axis() self._check_series_display((False, True)) + self._check_sort_by() return self + def several_series(self) -> bool: + return bool(self.groupby) or len(self.metrics) > 1 + + def sort_metric(self) -> str | None: + """The metric sort_by ranks by, or None (unset, or "total").""" + return self.sort_by if self.sort_by not in (None, self.TOTAL) else None + + def _check_sort_by(self) -> None: + if self.sort_by is None: + return + name = self.name + if self.category_sort: + raise ValueError(f"chart {name!r}: set category_sort or sort_by, not both " + "(one orders the bars by label, the other by a measure)") + if self.contribution: + raise ValueError(f"chart {name!r}: sort_by ranks by values, and contribution plots " + "shares (a sort metric would count in each bar's share)") + if self.sort_by == self.TOTAL: + if not self.several_series(): + raise ValueError(f"chart {name!r}: sort_by \"total\" ranks several series by " + "their sum; one series already ranks by its metric") + elif self.groupby: + # sortOperator.ts skips a chart with a groupby (4.1.4 and 5.0.0 :46, 6.1.0 :45): + # the plugin then sorts the categories by name or by a sum, min, max or mean + # of the series (SortSeriesType), never by one metric. + raise ValueError(f"chart {name!r}: Superset ranks grouped bars by their series " + f"only; use sort_by \"total\", or drop the groupby to rank by " + f"{self.sort_by!r}") + class PieChart(_Legend, _ChartBase, _ColorSchemeMixin): type: Literal["pie"] @@ -920,6 +969,31 @@ class HeatmapChart(_ChartBase): "whole heatmap (default), its x value's column, or its y value's row", ) show_legend: bool = Field(default=True, description="false hides the colour scale") + # Heatmap controlPanel.tsx xscale_interval / yscale_interval (-1 auto, 1-50) and + # left_margin ('auto' or px, free-form) at 4.1.4, 5.0.0 and 6.1.0. + x_label_every: int | None = Field( + default=None, ge=1, le=50, + description="A label every N x values, counted from the first: 6 on an hour axis " + "reads 0, 6, 12, 18, and 1 (or true) labels every value. Omit for " + "Superset's automatic spacing, which drops labels that would collide", + ) + y_label_every: int | None = Field( + default=None, ge=1, le=50, + description="As x_label_every, counted up the y axis from its bottom label (ECharts steps from index 0, the bottom): with y_order \"a_to_z\", whose first label is on top, a step of 6 on 24 hours labels 23, 17, 11, 05; leave y_order unset to read 00, 06, 12, 18 upward") + left_margin: int | None = Field( + default=None, ge=0, le=200, + description="Room, in px, left of the y-axis labels. Superset 6.1.0 draws them wider " + "than it measures them and cuts the longest label's first letters off " + "at the card's edge; 16 clears them. Omit for none. Leave it unset unless " + "asked: `advise --fix` fills it", + ) + + @field_validator("x_label_every", "y_label_every", mode="before") + @classmethod + def _label_every(cls, v): + # A bar's x_label_every is true or false; here true is 1 (every value) and + # false is Superset's automatic spacing, so both spellings read the same. + return None if v is False else v class HistogramChart(_ChartBase, _ColorSchemeMixin): @@ -973,7 +1047,9 @@ class TreemapChart(_ChartBase, _ColorSchemeMixin): class MixedSeries(BaseModel): """One of a mixed chart's two queries: its metrics, drawn as bars or a line, on - the primary (left) or secondary (right) value axis.""" + the primary (left) or secondary (right) value axis. No release gives a query's + line a width or dash of its own (docs/CONTRACTS.md): set a ghost line apart by + colour, with dashboard.label_colors on its series label.""" model_config = ConfigDict(extra="forbid") @@ -1638,8 +1714,9 @@ def _hex_colours(self) -> "DashboardMeta": # The chart fields the design brain may fill with a design default (`advise --fix`, # docs/DESIGN-BRAIN.md section 16). design.filled may list only these. -BRAIN_FILLABLE_FIELDS = ("cell_bars", "compare_suffix", "number_format", "page_length", - "search_box", "show_legend", "show_value", "x_label_format") +BRAIN_FILLABLE_FIELDS = ("cell_bars", "compare_suffix", "left_margin", "number_format", + "page_length", "search_box", "show_legend", "show_value", + "x_label_format") # design.standard_written: one entry per item of spec content a standard wrote @@ -2116,7 +2193,8 @@ def resolved_height(self, name: str) -> float: def chart_metrics(chart) -> list[str]: """Every metric string a chart names: its metric(s), a mixed chart's two - queries, an aggregate table's sort metric and the metric ranking a series limit.""" + queries, an aggregate table's or a bar's sort metric and the metric ranking a + series limit.""" out: list[str] = [] if getattr(chart, "metric", None): out.append(chart.metric) @@ -2128,6 +2206,8 @@ def chart_metrics(chart) -> list[str]: out += [s.series_limit_metric for s in (chart.a, chart.b) if s.series_limit_metric] if chart.type == "table" and chart.sort_by and not chart.columns: out.append(chart.sort_by) + if chart.type == "bar" and chart.sort_metric(): + out.append(chart.sort_metric()) return out diff --git a/chartwright/versions.py b/chartwright/versions.py index 1a1a2af..078c0e5 100644 --- a/chartwright/versions.py +++ b/chartwright/versions.py @@ -112,7 +112,9 @@ def _charts(predicate) -> Callable[[DashboardSpec], list[str | None]]: "force_max_interval is a 6.1.0 control (controls.tsx:389, Timeseries and " "MixedTimeseries controlPanel.tsx); xAxisLabelInterval is at 6.0.0 controls.tsx:305; " "neither at 4.1.4 or 5.0.0", - _charts(lambda c: getattr(c, "x_label_every", False)), + # A heatmap's x_label_every is its own xscale_interval, which every release reads + # (Heatmap/controlPanel.tsx 4.1.4 :130, 5.0.0 :128, 6.1.0 :155). + _charts(lambda c: c.type != "heatmap" and getattr(c, "x_label_every", False)), ), GatedField( "subtitle", "6.0.0", "warn", diff --git a/docs/CONTRACTS.md b/docs/CONTRACTS.md index ecc6fae..dd5d2b7 100644 --- a/docs/CONTRACTS.md +++ b/docs/CONTRACTS.md @@ -323,7 +323,29 @@ running the tool against real instances of all three releases. (`plugin-chart-echarts/src/Timeseries/transformers.ts`, 4.1.4 `:356`, 5.0.0 `:362`, 6.1.0 `:457`, the same body in each). Its colour, opacity, width and line style (solid, dashed or dotted) reach the chart; a formula - layer has no on-chart label, so the tool offers none. + layer has no on-chart label, so the tool offers none. On a horizontal bar + each point becomes `[y, x]` (4.1.4 `:381-383`, 5.0.0 `:387-389`, 6.1.0 + `:482-484`), so a `value` lands on the value axis, which runs across, and + the line stands upright at it: a 1.0x threshold or a 4-hour limit over a + ranked list. Seen live on all three releases. On either orientation the + line runs from the first category to the last, not the full plot. +- **A mixed chart's lines take no width or dash of their own, on any + release.** Each query's display controls are its series type, stacking, + area fill and that fill's opacity, values on the marks, markers and their + size, and its axis (`createCustomizeSection` in + `MixedTimeseries/controlPanel.tsx`, 4.1.4 and 5.0.0 `:132-262`; 6.1.0 + `:137-313` adds `only_total` and a series sort). Neither query gets a line + style from `transformProps` (`transformSeries` for queries A and B, + 4.1.4 `:382,429`, 5.0.0 `:384,432`, 6.1.0 `:457,530`). The one-query + Timeseries chart dashes only time-comparison series + (`Timeseries/transformProps.ts`, 4.1.4 `:285-296`, 6.1.0 `:400-422`), and + the mixed chart has no such pass. 6.1.0's "ECharts Options" can't style one + series either: the merge replaces arrays whole + (`utils/mergeCustomEChartOptions.ts:70-72`), so a `series` entry would drop + the chart's own. The spec therefore has no line style for a mixed query. + Tell a ghost line from the ink line by colour: `dashboard.label_colors` + pins a pale grey to the ghost's series label. A fixed level can be an + annotation, which does take `style`, `width` and `opacity`. - **Options genuinely differ by release.** 6.0.0 renamed the big-number subtitle field (`subheader` became `subtitle`) and removed sort controls that older releases still have. The tool emits only options valid on all @@ -387,6 +409,40 @@ running the tool against real instances of all three releases. 6.1.0 `:335`) and `x_axis_sort_series` at 4.1.4 and 5.0.0 (`:243` at 4.1.4, `:246` at 5.0.0). `category_sort` writes whichever applies. +- **A bar ranks by a measure two ways, and its default ranking holds for one + series only.** Unsorted by category, a bar writes its first metric as + `x_axis_sort`. With one series, or several metrics and no groupby on 6.0.0 + and later, that ranks the bars by it. With several series the plugin + re-sorts the rows itself (`sortRows`, `utils/series.ts`): at 4.1.4 and + 5.0.0 by `x_axis_sort_series`, whose panel default is the category name + (`DEFAULT_XAXIS_SORT_SERIES_DATA`, `constants.ts:77-80`), which a dashboard + fills in for an unset key; with a groupby, `sortOperator.ts` adds no sort + on any release and the categories keep the order the pivot gives them + (pandas `pivot_table`, which sorts its index: + `superset/utils/pandas_postprocessing/pivot.py:92` at 6.1.0). Seen + live: two stacked metrics read A to Z on 4.1.4, and a grouped bar A to Z + on all three releases. `sort_by` ranks them on purpose: + - `"total"` is Superset's "Total value" (`SortSeriesType.Sum`), the sum of + each category's series (`sortRows`, 4.1.4 `:200-208`, 6.1.0 `:564-572`), + read from `x_axis_sort` from 6.0.0 (`Timeseries/transformProps.ts:335-336` + at 6.1.0) and `x_axis_sort_series` at 4.1.4 and 5.0.0 (`:243-246`, + `:246-249`); the tool writes both. The query stays ordered by the first + metric, so a row limit cuts by it before the bars are ranked, and + `advise` warns (`data.top-n-sort`). + - A metric is the post-processing sort (`sortOperator.ts`, which needs no + groupby) and the query's "Sort query by" (`timeseries_limit_metric`), so + a row limit keeps the top bars by it. A metric the chart doesn't draw is + queried but not drawn: `extractExtraMetrics.ts:35` adds it because its + label is `x_axis_sort`, and `extractSeries` leaves it out of the series. + With several metrics, 4.1.4 and 5.0.0 would then re-sort by the name + default, so the tool writes `x_axis_sort_series: null`, which + `extractSeries` reads as no sort (`isDefined`, 4.1.4 `:296-307`, 5.0.0 + `:302-313`); 6.0.0 dropped the control. + Verified live on 4.1.4, 5.0.0 and 6.1.0, with `plan` clean after each + apply. A grouped bar can't rank by one metric (the plugin sorts by name, + sum, minimum, maximum or mean of the series), so `sort_by` refuses a + metric there; Superset's minimum, maximum and average have no spec value, + and `decompile` names them, and a ranking reversed to smallest first. - **Which rows a bar's row limit keeps is set by the query's ORDER BY, not by that sort.** The bar's query takes its ORDER BY from `normalizeOrderBy` (`Timeseries/buildQuery.ts:93` at all three releases): the "Sort query by" @@ -410,6 +466,24 @@ running the tool against real instances of all three releases. With `series_limit` the same control ranks the series, so the tool leaves it to the series limit; there a `row_limit` still keeps the largest values, and `advise` warns about it (`data.top-n-sort`). +- **A heatmap's axes take the same three controls on every release.** + `xscale_interval` and `yscale_interval` (-1 for automatic, 1 to 50) and the + free-form `left_margin` (`Heatmap/controlPanel.tsx`, 4.1.4 `:130,148,166`, + 5.0.0 `:128,146,164`, 6.1.0 `:155,173,191`). The plugin hands interval + N - 1 to the category axis, so N labels every Nth value from the first + (`Heatmap/transformProps.ts`, 4.1.4 `:231,238`, 5.0.0 `:234,241`, 6.1.0 + `:436,445`); `x_label_every` and `y_label_every` write them, with no + release gate. The grid holds its labels (`containLabel`) and its left + edge is `left_margin`, `auto` being the card's edge (4.1.4 `:166-169`, + 6.1.0 `:354-357`). On 6.1.0 the longest y label is drawn wider than that + room and loses its first letters at the edge; seen live, 8 px cleared a + 95 px label and 16 px a 150 px one, while 4.1.4 and 5.0.0 drew them whole. + A theme can't reach it: 6.1.0's `Heatmap.tsx:25` renders `` + without `vizType`, so `echartsOptionsOverridesByChartType` never applies + (`components/Echart.tsx:255-257`). `left_margin` is the spec's field for + it, and `advise --fix` fills 16 (`default.heatmap-label-room`). + `bottom_margin` is declared too but nothing was seen to need it, so the + spec leaves it to Superset, and `decompile` names a changed one. - **On 4.1.4, heatmap and histogram exist twice** (a legacy plugin and a current one, with different options). The tool builds the current ones; decompiling a dashboard built on the legacy ones reports them as named diff --git a/docs/DESIGN-BRAIN.md b/docs/DESIGN-BRAIN.md index 73d434c..0081c73 100644 --- a/docs/DESIGN-BRAIN.md +++ b/docs/DESIGN-BRAIN.md @@ -1,8 +1,9 @@ # The Design Brain -> **Status: SHIPPED, design brain 7** (4 added `narrative.color-scheme`; 5 the +> **Status: SHIPPED, design brain 12** (4 added `narrative.color-scheme`; 5 the > design defaults of §16; 6 the `standard.*` rules of §18's content; 7 fills keeping -> a null record after an edit, and `default.stale-record`). This page is both the design and the +> a null record after an edit, and `default.stale-record`; 12 `default.heatmap-label-room`, +> and `data.top-n-sort` on a bar ranked by `sort_by: "total"`). This page is both the design and the > reference for the implementation in `chartwright/design/`. The decision log > at the bottom records every judgment call made without a review gate; §15 > records where the implementation deliberately deviates from the design @@ -327,6 +328,7 @@ fails when it drifts. | `default.cell-bars` | info | ✔ | - | 5 | a raw table with id, code, year or zip columns draws no cell bars (a bar behind an identifier reads as an amount) | | `default.compare-suffix` | info | ✔ | - | 5 | a trendline KPI's change says what it compares against ('vs previous month') | | `default.count-format` | info | ✔ | - | 5 | counts read as whole numbers with thousands separators (',.0f') | +| `default.heatmap-label-room` | info | ✔ | - | 12 | a heatmap keeps 16 px left of its y labels, where Superset 6.1.0 cuts off the longest one's first letters | | `default.page-length` | info | ✔ | - | 5 | a table whose row_limit outgrows its panel pages by the rows that fit beside its page controls | | `default.search-box` | info | ✔ | - | 5 | a raw table of more than ~20 rows gets a search box, when its rows still fit beside it (the 20 is judgement) | | `default.single-series-legend` | info | ✔ | - | 5 | a single series named by the chart or y-axis title needs no legend | @@ -1077,6 +1079,7 @@ thresholds; those marked judgement are usability choices, not pixel facts. | `default.search-box` | `search_box` | `true` | a raw-mode table with an explicit `row_limit` above `search_min_rows` (20, judgement), when the bar the box sits in hides no row: a paged table already draws it, and a table on one page must still fit every row beside it | | `default.single-series-legend` | `show_legend` | `false` | a timeseries chart or categorical bar with one metric, no groupby, no series limit, no goal lines and no legend placement written, whose shown title or `y_axis_title` contains the metric's label, and that label is at least 3 characters long. Never a heatmap, whose legend is the colour scale | | `default.value-labels` | `show_value` | `true` | a categorical bar with one metric, no groupby, no `contribution`, an explicit `row_limit` of at most `value_label_max_bars` (12). A vertical bar also needs a width of at least `value_label_min_width` (6/12); a horizontal bar needs the height to space its labels, 4.5 units plus 0.4125 a bar (§17) | +| `default.heatmap-label-room` | `left_margin` | `16` | every heatmap (brain 12). With no margin, a heatmap's grid starts at the card's edge, and Superset 6.1.0 draws the longest y label wider than the room the grid gave it, cutting off its first letters: seen live on `Trucks and Buses` (95 px) and `Australian Gift Network, Co` (150 px). 8 px cleared the first and 16 px both; 4.1.4 and 5.0.0 drew them whole and only gain 16 px of space. A theme can't fix it, since 6.1.0 renders the heatmap without its chart type and no per-type override reaches it | `narrative.big-number-format` stands down where `default.count-format` offers the same remedy with a fix: one remedy, one finding. diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 57bbf30..76b5887 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -30,13 +30,13 @@ into one. Everything below works from that one file. - Axes: titles, a fixed floor or ceiling, a log scale; a mixed chart's second axis takes its own. - Values written on bars and points, stacked series, 100% stacks, and the top N series of a breakdown. - Legends hidden, or placed at the bottom, left or right. - - Bars in category order (hours, ranks, `1-Mon` weekdays) instead of by value. + - Bars in category order (hours, ranks, `1-Mon` weekdays) instead of by value; stacked or grouped bars ranked by their total; or bars ranked by another metric, drawn or not, so two charts share one order. - A time range per chart, such as a "Last 30 days" KPI on a dashboard that shows all time. - A trendline KPI's change against an earlier period ("+4% vs last month"), its line colour, and on Superset 6.0+ a subtitle. - Tables: page size, a totals row, a search box, column alignment and widths, and on Superset 6.0+ header names. - Pivots: averages and other aggregations, rows sorted by value, row subtotals, rows and columns swapped. - - Heatmap values and colour scheme; what pie, funnel and treemap labels show, and their number format. -- **Goal Lines**: Draw a target or trend line over line, bar, area, scatter and mixed charts with `annotations`: `{"name": "Goal", "value": 80, "style": "dashed"}`, or a formula in x for a trend. + - Heatmap values and colour scheme, a label on every Nth row or column (hours read 0, 6, 12, 18), and room left of the row labels, which Superset 6.1 otherwise cuts off; what pie, funnel and treemap labels show, and their number format. +- **Goal Lines**: Draw a target or trend line over line, bar, area, scatter and mixed charts with `annotations`: `{"name": "Goal", "value": 80, "style": "dashed"}`, or a formula in x for a trend. On a horizontal bar the line stands upright at its value, such as a 1.0x threshold across a ranked list. - **Dashboard and Chart Settings**: Set the dashboard's own settings and each chart's in the spec, so a change made in the UI shows up in `plan`. - Dashboard: colour scheme, description, certification badge, draft state, auto-refresh interval, and the filter bar across the top (on 4.1.4 and 5.0.0, with Superset's `HORIZONTAL_FILTER_BAR` flag on). - Chart: colour scheme, description (viewers open it from the chart menu), certification badge, cache timeout, and a shorter title shown on the dashboard. diff --git a/docs/VERIFICATION.md b/docs/VERIFICATION.md index e22865b..cdd0f29 100644 --- a/docs/VERIFICATION.md +++ b/docs/VERIFICATION.md @@ -35,7 +35,7 @@ injection. | Layer | Proves | Where it runs | |---|---|---| -| Offline suite (1564 tests, 67 modules) | Contract, determinism, round-trips, credentials | every PR and push to main, Linux + Windows, mcp 1 and 2 | +| Offline suite (1594 tests, 67 modules) | Contract, determinism, round-trips, credentials | every PR and push to main, Linux + Windows, mcp 1 and 2 | | Chart-option contract | Every emitted chart option is declared by each version's plugin source | every PR and push to main | | Live guarantee check | Three specs applied (every chart type, every display control, every dashboard and filter control), per-chart data check, ids stable across re-apply | every PR and push to main, all 3 versions | | Standards content round trip | `standards apply` writes an org footer, a team header and two CSS blocks into a spec; it applies, `plan` stays clean, decompile reads the CSS markers and every managed row back, `--claim` rebuilds the record, re-apply stays clean | every PR and push to main, all 3 versions | diff --git a/schema/dashboard_spec.schema.json b/schema/dashboard_spec.schema.json index 98f9876..2209230 100644 --- a/schema/dashboard_spec.schema.json +++ b/schema/dashboard_spec.schema.json @@ -33,7 +33,7 @@ }, "Annotation": { "additionalProperties": false, - "description": "A FORMULA annotation layer: a line drawn from a formula in x, e.g. a goal\nline at y = 80. Superset draws it as a series named ``name`` (in the legend\nand tooltip); formula layers have no on-chart label in any release.", + "description": "A FORMULA annotation layer: a line drawn from a formula in x, e.g. a goal\nline at y = 80. Superset draws it as a series named ``name`` (in the legend\nand tooltip); formula layers have no on-chart label in any release. On a\nhorizontal bar the value axis runs across, so the line stands upright at its\nvalue (transformFormulaAnnotation swaps the point to [y, x] there).", "properties": { "name": { "description": "Series name in the legend and tooltip, e.g. \"Goal\"", @@ -51,7 +51,7 @@ } ], "default": null, - "description": "A flat line at this y value, e.g. 80", + "description": "A line at this value on the value axis, e.g. 80: flat across most charts, upright across a horizontal bar (a 1.0x threshold, a 4-hour limit)", "title": "Value" }, "formula": { @@ -387,7 +387,7 @@ "title": "X Label Rotation" }, "annotations": { - "description": "Formula lines over the chart, e.g. a goal: [{\"name\": \"Goal\", \"value\": 80, \"style\": \"dashed\"}] (Superset's FORMULA annotation layers)", + "description": "Formula lines over the chart, e.g. a goal: [{\"name\": \"Goal\", \"value\": 80, \"style\": \"dashed\"}] (Superset's FORMULA annotation layers); on a horizontal bar a value stands upright across the bars", "items": { "$ref": "#/$defs/Annotation" }, @@ -595,6 +595,20 @@ "default": null, "description": "Order the bars by their category (x_column) instead of by the first metric: \"asc\" reads A to Z or 0 to 9 (hours, ranks, '1-Mon'), left to right or top to bottom on a horizontal bar; \"desc\" reverses it. The query is ordered by the category too, so a row_limit keeps the first categories in that order, not the largest values. With series_limit the query stays ordered by the series ranking (Superset has one \"Sort query by\" control for both), so there a row_limit keeps the largest values", "title": "Category Sort" + }, + "sort_by": { + "anyOf": [ + { + "minLength": 1, + "type": "string" + }, + { + "type": "null" + } + ], + "default": null, + "description": "Rank the bars by something other than the first metric, largest first (on the left, or on top of a horizontal bar). \"total\" ranks stacked or grouped bars by their sum; a metric, written like any metric, ranks them by its value and orders the query by it, so a row_limit keeps the top bars by it. The metric need not be drawn: two charts sorted by the same one share an order. A metric needs no groupby; with \"total\" a row_limit keeps the rows with the largest first metric, so for a top N by total write the total as a metric, e.g. \"SQL(SUM(a) + SUM(b)) AS Total\"", + "title": "Sort By" } }, "required": [ @@ -2214,6 +2228,51 @@ "description": "false hides the colour scale", "title": "Show Legend", "type": "boolean" + }, + "x_label_every": { + "anyOf": [ + { + "maximum": 50, + "minimum": 1, + "type": "integer" + }, + { + "type": "null" + } + ], + "default": null, + "description": "A label every N x values, counted from the first: 6 on an hour axis reads 0, 6, 12, 18, and 1 (or true) labels every value. Omit for Superset's automatic spacing, which drops labels that would collide", + "title": "X Label Every" + }, + "y_label_every": { + "anyOf": [ + { + "maximum": 50, + "minimum": 1, + "type": "integer" + }, + { + "type": "null" + } + ], + "default": null, + "description": "As x_label_every, counted up the y axis from its bottom label (ECharts steps from index 0, the bottom): with y_order \"a_to_z\", whose first label is on top, a step of 6 on 24 hours labels 23, 17, 11, 05; leave y_order unset to read 00, 06, 12, 18 upward", + "title": "Y Label Every" + }, + "left_margin": { + "anyOf": [ + { + "maximum": 200, + "minimum": 0, + "type": "integer" + }, + { + "type": "null" + } + ], + "default": null, + "description": "Room, in px, left of the y-axis labels. Superset 6.1.0 draws them wider than it measures them and cuts the longest label's first letters off at the card's edge; 16 clears them. Omit for none. Leave it unset unless asked: `advise --fix` fills it", + "title": "Left Margin" } }, "required": [ @@ -3008,7 +3067,7 @@ "title": "X Label Rotation" }, "annotations": { - "description": "Formula lines over the chart, e.g. a goal: [{\"name\": \"Goal\", \"value\": 80, \"style\": \"dashed\"}] (Superset's FORMULA annotation layers)", + "description": "Formula lines over the chart, e.g. a goal: [{\"name\": \"Goal\", \"value\": 80, \"style\": \"dashed\"}] (Superset's FORMULA annotation layers); on a horizontal bar a value stands upright across the bars", "items": { "$ref": "#/$defs/Annotation" }, @@ -3183,7 +3242,7 @@ }, "MixedSeries": { "additionalProperties": false, - "description": "One of a mixed chart's two queries: its metrics, drawn as bars or a line, on\nthe primary (left) or secondary (right) value axis.", + "description": "One of a mixed chart's two queries: its metrics, drawn as bars or a line, on\nthe primary (left) or secondary (right) value axis. No release gives a query's\nline a width or dash of its own (docs/CONTRACTS.md): set a ghost line apart by\ncolour, with dashboard.label_colors on its series label.", "properties": { "metrics": { "items": { @@ -5203,7 +5262,7 @@ "title": "X Label Rotation" }, "annotations": { - "description": "Formula lines over the chart, e.g. a goal: [{\"name\": \"Goal\", \"value\": 80, \"style\": \"dashed\"}] (Superset's FORMULA annotation layers)", + "description": "Formula lines over the chart, e.g. a goal: [{\"name\": \"Goal\", \"value\": 80, \"style\": \"dashed\"}] (Superset's FORMULA annotation layers); on a horizontal bar a value stands upright across the bars", "items": { "$ref": "#/$defs/Annotation" }, @@ -5708,7 +5767,7 @@ "title": "X Label Rotation" }, "annotations": { - "description": "Formula lines over the chart, e.g. a goal: [{\"name\": \"Goal\", \"value\": 80, \"style\": \"dashed\"}] (Superset's FORMULA annotation layers)", + "description": "Formula lines over the chart, e.g. a goal: [{\"name\": \"Goal\", \"value\": 80, \"style\": \"dashed\"}] (Superset's FORMULA annotation layers); on a horizontal bar a value stands upright across the bars", "items": { "$ref": "#/$defs/Annotation" }, @@ -6177,7 +6236,7 @@ "title": "X Label Rotation" }, "annotations": { - "description": "Formula lines over the chart, e.g. a goal: [{\"name\": \"Goal\", \"value\": 80, \"style\": \"dashed\"}] (Superset's FORMULA annotation layers)", + "description": "Formula lines over the chart, e.g. a goal: [{\"name\": \"Goal\", \"value\": 80, \"style\": \"dashed\"}] (Superset's FORMULA annotation layers); on a horizontal bar a value stands upright across the bars", "items": { "$ref": "#/$defs/Annotation" }, @@ -6688,7 +6747,7 @@ "title": "X Label Rotation" }, "annotations": { - "description": "Formula lines over the chart, e.g. a goal: [{\"name\": \"Goal\", \"value\": 80, \"style\": \"dashed\"}] (Superset's FORMULA annotation layers)", + "description": "Formula lines over the chart, e.g. a goal: [{\"name\": \"Goal\", \"value\": 80, \"style\": \"dashed\"}] (Superset's FORMULA annotation layers); on a horizontal bar a value stands upright across the bars", "items": { "$ref": "#/$defs/Annotation" }, diff --git a/tests/fixtures/display_controls.json b/tests/fixtures/display_controls.json index ab8f2d5..8a566c9 100644 --- a/tests/fixtures/display_controls.json +++ b/tests/fixtures/display_controls.json @@ -102,6 +102,27 @@ "legend_position": "right", "time_range": "Last year" }, + { + "name": "Countries by Deal Size", + "type": "bar", + "dataset": {"database": "examples", "table": "cleaned_sales_data"}, + "x_column": "country", + "metrics": ["SUM(sales)"], + "groupby": "deal_size", + "orientation": "horizontal", + "stack": true, + "sort_by": "total" + }, + { + "name": "Lines by Quantity", + "type": "bar", + "dataset": {"database": "examples", "table": "cleaned_sales_data"}, + "x_column": "product_line", + "metrics": ["SUM(sales)"], + "orientation": "horizontal", + "sort_by": "SUM(quantity_ordered)", + "annotations": [{"name": "Target", "value": 1000000, "style": "dashed", "color": "#B3261E"}] + }, { "name": "Share by Line", "type": "pie", @@ -179,7 +200,10 @@ "number_format": ",.0f", "show_percentage": false, "normalize_across": "y", - "show_legend": false + "show_legend": false, + "x_label_every": 2, + "y_label_every": 1, + "left_margin": 16 }, { "name": "Price Spread", @@ -217,6 +241,7 @@ ["Sales by Line", "Monthly Orders"], ["Quantity Area", "Price vs Time"], ["Orders by Hour", "Share by Line"], + ["Countries by Deal Size", "Lines by Quantity"], ["Order Funnel", "Sales Treemap"], ["Top Customers"], ["Sales Pivot"], diff --git a/tests/fixtures/live_display_controls.json b/tests/fixtures/live_display_controls.json index 710963d..381374a 100644 --- a/tests/fixtures/live_display_controls.json +++ b/tests/fixtures/live_display_controls.json @@ -102,6 +102,27 @@ "legend_position": "right", "time_range": "Last year" }, + { + "name": "Countries by Deal Size", + "type": "bar", + "dataset": {"database": "examples", "table": "cleaned_sales_data"}, + "x_column": "country", + "metrics": ["SUM(sales)"], + "groupby": "deal_size", + "orientation": "horizontal", + "stack": true, + "sort_by": "total" + }, + { + "name": "Lines by Quantity", + "type": "bar", + "dataset": {"database": "examples", "table": "cleaned_sales_data"}, + "x_column": "product_line", + "metrics": ["SUM(sales)"], + "orientation": "horizontal", + "sort_by": "SUM(quantity_ordered)", + "annotations": [{"name": "Target", "value": 1000000, "style": "dashed", "color": "#B3261E"}] + }, { "name": "Share by Line", "type": "pie", @@ -179,7 +200,10 @@ "number_format": ",.0f", "show_percentage": false, "normalize_across": "y", - "show_legend": false + "show_legend": false, + "x_label_every": 2, + "y_label_every": 1, + "left_margin": 16 }, { "name": "Price Spread", @@ -217,6 +241,7 @@ ["Sales by Line", "Monthly Orders"], ["Quantity Area", "Price vs Time"], ["Orders by Hour", "Share by Line"], + ["Countries by Deal Size", "Lines by Quantity"], ["Order Funnel", "Sales Treemap"], ["Top Customers"], ["Sales Pivot"], diff --git a/tests/test_annotations.py b/tests/test_annotations.py index c55f47d..0aa5bdb 100644 --- a/tests/test_annotations.py +++ b/tests/test_annotations.py @@ -46,6 +46,21 @@ def test_a_goal_line_compiles_to_a_formula_layer(chart_type): } +def test_a_threshold_on_a_horizontal_bar_is_the_same_layer_drawn_upright(): + """transformFormulaAnnotation maps each point to [y, x] on a horizontal chart + (Timeseries/transformers.ts, 4.1.4 :381-383, 5.0.0 :387-389, 6.1.0 :482-484), so a + value lands on the value axis, which runs across: the line stands upright at 1.0x. + Seen live on 4.1.4, 5.0.0 and 6.1.0. The layer itself is unchanged.""" + threshold = {"name": "Break-even", "value": 1, "style": "dashed", "color": "#B3261E"} + data = one_chart("bar", annotations=[threshold], orientation="horizontal") + p = chart_doc(load_spec(data), "Trend")["params"] + assert p["orientation"] == "horizontal" + vertical = layers(one_chart("bar", annotations=[threshold])) + assert p["annotation_layers"] == vertical and vertical[0]["value"] == "1" + out = assert_lossless(load_spec(data)) + assert out.spec["charts"][0]["annotations"] == [threshold] + + def test_a_formula_and_the_optional_settings(): (layer,) = layers(one_chart(annotations=[ {"name": "Trend line", "formula": " 0.5*x + 3 ", "style": "dotted", "opacity": "low", "width": 1.5}])) diff --git a/tests/test_decompile.py b/tests/test_decompile.py index 53283f8..9326dbf 100644 --- a/tests/test_decompile.py +++ b/tests/test_decompile.py @@ -263,7 +263,7 @@ def text_row(path, doc): ("pie", "outerRadius", 60), ("pie", "show_labels", False), ("funnel", "tooltip_label_type", 2), ("funnel", "show_labels", False), ("treemap_v2", "show_labels", False), - ("heatmap_v2", "legend_type", "piecewise"), ("heatmap_v2", "xscale_interval", 2), + ("heatmap_v2", "legend_type", "piecewise"), ("heatmap_v2", "bottom_margin", 50), ("histogram_v2", "normalize", True), ("big_number", "show_trend_line", False), ("echarts_timeseries_line", "seriesType", "smooth"), ("echarts_timeseries_line", "currency_format", {"symbol": "USD", "symbolPosition": "prefix"}), diff --git a/tests/test_design_defaults.py b/tests/test_design_defaults.py index 105e4e6..35d4c82 100644 --- a/tests/test_design_defaults.py +++ b/tests/test_design_defaults.py @@ -90,7 +90,7 @@ def test_every_fill_is_info_fixable_and_governs_a_fillable_field(): for rid, fill in FILLS.items(): r = RULES[rid] assert rid.startswith("default.") and r.severity == "info" and r.fixable - assert r.since == "5" + assert r.since == ("12" if rid == "default.heatmap-label-room" else "5") @pytest.mark.parametrize("path", FIXTURES, ids=lambda p: p.name) @@ -414,6 +414,39 @@ def test_vertical_value_labels_stand_down_below_six_twelfths(): assert filled_value(data, "default.value-labels", "B") is None +# -- default.heatmap-label-room ---------------------------------------------------------- + + +def heatmap(name="H", **kw): + return {"type": "heatmap", "name": name, "dataset": DS, "x_column": "hour", + "y_column": "zone", "metric": "SUM(x)", "number_format": ",.0f", **kw} + + +def test_heatmap_label_room_fills_16_px_and_leaves_the_authors_value(): + """Seen on 6.1.0: the longest y label lost its first letters at the card's edge with + no margin (a 95 px label at 8 px clear, a 150 px one at 16 px).""" + assert filled_value(mk([heatmap()]), "default.heatmap-label-room", "H") == 16 + assert fills(mk([heatmap(left_margin=0)]), "default.heatmap-label-room") == {} + assert fills(mk([bar()]), "default.heatmap-label-room") == {} + fixed, rep = fix(mk([heatmap()])) + assert fixed["charts"][0]["left_margin"] == 16 + assert fixed["design"] == {"filled": {"H": {"left_margin": 16}}} + params = next(p for p in _compiled_params(fixed).values()) + assert params["left_margin"] == 16 + + +def _compiled_params(data) -> dict[str, dict]: + import io + import zipfile + + import yaml + + spec = load_spec(data) + zf = zipfile.ZipFile(io.BytesIO(compile_bundle(spec, stub_resolution(spec)))) + return {cy["slice_name"]: cy["params"] for cy in + (yaml.safe_load(zf.read(n)) for n in zf.namelist() if "/charts/" in n)} + + # -- provenance: design.filled ------------------------------------------------------------- @@ -755,7 +788,7 @@ def test_explain_shows_every_governed_field_and_its_source(monkeypatch, tmp_path code, out = _cli(["explain", str(spec), "--json", "--chart", "A"], monkeypatch, tmp_path, capsys) assert code == 0 and json.loads(out)["charts"][0]["chart"] == "A" code, out = _cli(["explain", str(spec)], monkeypatch, tmp_path, capsys) - assert code == 0 and out.startswith("Design defaults (design brain 7") + assert code == 0 and out.startswith("Design defaults (design brain 12") code, out = _cli(["explain", str(spec), "--chart", "Z"], monkeypatch, tmp_path, capsys) assert code == 1 and "unknown_chart" in out diff --git a/tests/test_design_display_controls.py b/tests/test_design_display_controls.py index ce39a83..9323c42 100644 --- a/tests/test_design_display_controls.py +++ b/tests/test_design_display_controls.py @@ -64,6 +64,29 @@ def test_a_row_limit_on_a_category_sorted_bar_with_a_series_limit_warns(): assert not _fired("data.top-n-sort", [no_limit]) +def test_a_row_limit_on_a_bar_ranked_by_total_warns_and_a_total_metric_does_not(): + """sort_by "total" ranks the rows the query returned (SortSeriesType.Sum); the query + is still ordered by the first metric, so the limit cuts by it. A total written as a + metric orders the query too.""" + bar = {"name": "Zones", "type": "bar", "dataset": DS, "x_column": "zone", + "metrics": ["SUM(first_4h)", "SUM(beyond_4h)"], "stack": True, + "orientation": "horizontal", "row_limit": 10, "height": 9, "sort_by": "total"} + (f,) = _fired("data.top-n-sort", [bar]) + assert "largest SUM(first_4h)" in f.detail and "sort_by it" in f.detail + grouped = {**bar, "metrics": ["SUM(x)"], "groupby": "duration"} + (f,) = _fired("data.top-n-sort", [grouped]) + assert "segments, not bars" in f.detail + assert not _fired("data.top-n-sort", [{**bar, "row_limit": None}]) + total = {**bar, "sort_by": "SQL(SUM(first_4h) + SUM(beyond_4h)) AS Total"} + assert not _fired("data.top-n-sort", [total]) + + +def test_a_bar_ranked_by_sort_by_is_no_ordinal_finding(): + bar = {**HOURS, "name": "By Weekday", "x_column": "weekday", "row_limit": 7, + "metrics": ["COUNT(*)", "SUM(x)"], "sort_by": "total"} + assert not _fired("chart.ordinal-order", [bar]) + + def test_a_paged_table_needs_room_for_one_page_not_every_row(): # Measured: at 12 units a paged table shows 9 rows on 6.1.0 (10 on 4.1.4 and 5.0.0), # so a 9-row page fits and a 10-row page hides its last row on 6.1.0. diff --git a/tests/test_display_controls.py b/tests/test_display_controls.py index a8a48bb..f261083 100644 --- a/tests/test_display_controls.py +++ b/tests/test_display_controls.py @@ -42,6 +42,7 @@ "compare_lag", "compare_suffix", "subtitle", "color_picker", "page_length", "show_totals", "include_search", "rowSubTotals", "transposePivot", "show_values", "show_percentage", "number_format", "show_total", "labels_outside", "markerEnabled", "area", + "xscale_interval", "yscale_interval", "left_margin", } @@ -110,6 +111,10 @@ def _display_params() -> dict[str, dict]: "stack": "Stack", "contributionMode": "column", "limit": 3, "legendOrientation": "right", "time_range": "Last year", "xAxisLabelInterval": "0"}), + ("Countries by Deal Size", {"x_axis_sort": "sum", "x_axis_sort_asc": True, + "x_axis_sort_series": "sum", + "x_axis_sort_series_ascending": True}), + ("Lines by Quantity", {"x_axis_sort": "SUM(quantity_ordered)", "x_axis_sort_asc": True}), ("Share by Line", {"label_type": "value_percent", "number_format": ",.0f", "show_total": True, "labels_outside": False, "legendOrientation": "left", "legendType": "plain"}), @@ -121,7 +126,8 @@ def _display_params() -> dict[str, dict]: "metricsLayout": "ROWS"}), ("Line by Deal Size", {"show_values": True, "linear_color_scheme": "schemeBlues", "y_axis_format": ",.0f", "show_percentage": False, - "normalize_across": "y", "show_legend": False}), + "normalize_across": "y", "show_legend": False, + "xscale_interval": 2, "yscale_interval": 1, "left_margin": 16}), ("Price Spread", {"x_axis_title": "Unit price", "y_axis_title": "Orders"}), ("Revenue KPI", {"compare_lag": 1, "compare_suffix": "vs last month", "subtitle": "Booked revenue", "time_range": "Last year", @@ -236,6 +242,86 @@ def test_a_grouped_bar_sorted_by_name_on_4_1_decompiles_as_category_sort(): assert "category_sort" not in _decompile(spec, edit).spec["charts"][0] +def test_sort_by_total_ranks_several_series_by_their_sum_on_every_release(): + """SortSeriesType.Sum: read from x_axis_sort from 6.0.0 and x_axis_sort_series at + 4.1.4 and 5.0.0, largest first (on top of the bottom-up horizontal axis). "sum" is + no label, so sortOperator adds no sort and the query keeps its own order.""" + bar = {"name": "B", "type": "bar", "dataset": DS, "x_column": "zone", + "metrics": ["SUM(first_4h)", "SUM(beyond_4h)"], "stack": True, "sort_by": "total"} + for orientation, asc in (("vertical", False), ("horizontal", True)): + for extra in ({}, {"metrics": ["SUM(x)"], "groupby": "region"}): + p = _params(_spec({**bar, **extra, "orientation": orientation}))["B"] + assert (p["x_axis_sort"], p["x_axis_sort_asc"]) == ("sum", asc) + assert (p["x_axis_sort_series"], p["x_axis_sort_series_ascending"]) == ("sum", asc) + assert "timeseries_limit_metric" not in p and "order_desc" not in p + contract = json.loads(CONTRACT.read_text(encoding="utf-8")) + for version in contract: + assert check(version, contract, {"echarts_timeseries_bar": set(p)}) == [], version + + +def test_sort_by_a_metric_sorts_and_orders_the_query_by_it_drawn_or_not(): + """sortOperator sorts the rows by x_axis_sort; as "Sort query by" the metric orders + the query (a row limit keeps the top bars by it), and one not drawn is queried as an + extra metric because its label is x_axis_sort (extractExtraMetrics.ts:35).""" + bar = {"name": "B", "type": "bar", "dataset": DS, "x_column": "zone", + "metrics": ["COUNT(*)"], "orientation": "horizontal", "row_limit": 10} + p = _params(_spec({**bar, "sort_by": "SUM(revenue)"}))["B"] + assert (p["x_axis_sort"], p["x_axis_sort_asc"]) == ("SUM(revenue)", True) + assert p["timeseries_limit_metric"]["label"] == "SUM(revenue)" + assert p["timeseries_limit_metric"]["column"] == {"column_name": "revenue"} + assert "order_desc" not in p and "x_axis_sort_series" not in p # descending: the top bars + # A drawn metric of several: at 4.1.4 and 5.0.0 the plugin would re-sort the rows by + # its x_axis_sort_series default (the name); null leaves sortOperator's order. + two = {**bar, "metrics": ["SUM(a)", "SUM(b)"], "sort_by": "SUM(b)", "orientation": "vertical"} + p = _params(_spec(two))["B"] + assert (p["x_axis_sort"], p["x_axis_sort_asc"]) == ("SUM(b)", False) + assert p["timeseries_limit_metric"] == p["metrics"][1] + assert "x_axis_sort_series" in p and p["x_axis_sort_series"] is None + contract = json.loads(CONTRACT.read_text(encoding="utf-8")) + for version in contract: + assert check(version, contract, {"echarts_timeseries_bar": set(p)}) == [], version + + +@pytest.mark.parametrize("spec_fields", [ + {"sort_by": "total", "metrics": ["SUM(a)", "SUM(b)"]}, + {"sort_by": "total", "groupby": "region", "series_limit": 3}, + {"sort_by": "SUM(hidden)"}, + {"sort_by": "SUM(b)", "metrics": ["SUM(a)", "SUM(b)"], "stack": True}, + {"sort_by": "COUNT(*)"}, +]) +def test_sort_by_round_trips_without_a_loss(spec_fields, monkeypatch): + bar = {"name": "B", "type": "bar", "dataset": DS, "x_column": "zone", "metrics": ["COUNT(*)"], + "orientation": "horizontal", **spec_fields} + spec = _spec(bar) + out = _decompile(spec) + assert out.losses == [], out.losses_json() + assert out.spec["charts"][0]["sort_by"] == spec_fields["sort_by"] + assert _normalize(load_spec(out.spec)) == _normalize(spec) + assert _plan(spec, None, monkeypatch)["clean"] is True + + +def test_a_bar_ranked_in_the_ui_decompiles_as_sort_by_or_a_named_loss(): + """4.1.4 and 5.0.0 store a several-series sort in x_axis_sort_series; Superset's + 'Total value' is sort_by "total", its minimum, maximum and average are no spec value, + and a ranking reversed to smallest first is named too.""" + spec = _spec({"name": "B", "type": "bar", "dataset": DS, "x_column": "zone", + "metrics": ["COUNT(*)"], "groupby": "region"}) + out = _decompile(spec, _edit_params("B", x_axis_sort=None, x_axis_sort_series="sum", + x_axis_sort_series_ascending=False)) + assert out.spec["charts"][0]["sort_by"] == "total" and out.losses == [] + out = _decompile(spec, _edit_params("B", x_axis_sort="max")) + assert "sort_by" not in out.spec["charts"][0] + assert any("'max'" in l.what for l in out.losses) + out = _decompile(spec, _edit_params("B", x_axis_sort="sum", x_axis_sort_asc=True)) + assert any("smallest first" in l.what for l in out.losses) + one = _spec({"name": "B", "type": "bar", "dataset": DS, "x_column": "zone", + "metrics": ["COUNT(*)"]}) + out = _decompile(one, _edit_params("B", x_axis_sort_asc=True)) # vertical: largest left + assert "sort_by" not in out.spec["charts"][0] + assert any("smallest first" in l.what for l in out.losses) + assert _decompile(one).losses == [] + + def test_y_axis_max_is_a_bound_on_every_release_not_echart_options(): line = {"name": "L", "type": "timeseries_line", "dataset": DS, "metrics": ["MAX(share)"], "time_column": "ts", "y_axis_max": 1} @@ -252,6 +338,56 @@ def test_heatmap_scheme_and_defaults_are_unchanged_when_omitted(): p = _params(_spec(hm))["H"] assert p["linear_color_scheme"] == "superset_seq_1" and p["normalize_across"] == "heatmap" assert "show_percentage" not in p and "show_values" not in p + assert not {"xscale_interval", "yscale_interval", "left_margin"} & set(p) + + +def test_heatmap_label_steps_and_left_margin(): + """xscale_interval / yscale_interval N label every Nth category from the first + (transformProps hands ECharts interval N - 1); left_margin is the grid's left edge. + The same controls at 4.1.4, 5.0.0 and 6.1.0 (Heatmap/controlPanel.tsx).""" + hm = {"name": "H", "type": "heatmap", "dataset": DS, "x_column": "hour", + "y_column": "weekday", "metric": "COUNT(*)"} + p = _params(_spec({**hm, "x_label_every": 6, "y_label_every": 1, "left_margin": 16}))["H"] + assert (p["xscale_interval"], p["yscale_interval"], p["left_margin"]) == (6, 1, 16) + # true labels every value, as on a bar; false is Superset's automatic spacing + spec = _spec({**hm, "x_label_every": True, "y_label_every": False}) + assert (spec.charts[0].x_label_every, spec.charts[0].y_label_every) == (1, None) + p = _params(spec)["H"] + assert p["xscale_interval"] == 1 and "yscale_interval" not in p + contract = json.loads(CONTRACT.read_text(encoding="utf-8")) + for version in contract: + assert check(version, contract, {"heatmap_v2": set(p)}) == [], version + + +@pytest.mark.parametrize("field, value, needle", [ + ("x_label_every", 0, "greater than or equal to 1"), + ("y_label_every", 51, "less than or equal to 50"), # the control's range, 1-50 + ("left_margin", -1, "greater than or equal to 0"), + ("left_margin", 201, "less than or equal to 200"), +]) +def test_heatmap_label_fields_hold_to_the_controls_range(field, value, needle): + hm = {"name": "H", "type": "heatmap", "dataset": DS, "x_column": "a", "y_column": "b", + "metric": "COUNT(*)", field: value} + with pytest.raises(ValidationError, match=needle): + _spec(hm) + + +def test_heatmap_label_fields_decompile_from_what_superset_stores(): + """The UI stores the interval as a number and a typed margin as text ('30'); + Superset's own -1 and 'auto' read as omitted, anything else is a named loss.""" + hm = {"name": "H", "type": "heatmap", "dataset": DS, "x_column": "a", "y_column": "b", + "metric": "COUNT(*)"} + spec = _spec(hm) + out = _decompile(spec, _edit_params("H", xscale_interval=3, yscale_interval=-1, + left_margin="30")) + chart = out.spec["charts"][0] + assert (chart["x_label_every"], chart["left_margin"]) == (3, 30) and "y_label_every" not in chart + assert out.losses == [] + out = _decompile(spec, _edit_params("H", xscale_interval=-1, left_margin="auto")) + assert not {"x_label_every", "left_margin"} & set(out.spec["charts"][0]) and out.losses == [] + out = _decompile(spec, _edit_params("H", xscale_interval=80, left_margin="5%")) + text = " ".join(l.what for l in out.losses) + assert "xscale_interval 80" in text and "left_margin '5%'" in text # -- version gating ------------------------------------------------------------------ @@ -393,6 +529,10 @@ def test_plan_is_clean_when_nothing_changed(monkeypatch): ("Top Customers", {"column_config": {}}), ("Sales Pivot", {"transposePivot": False}), ("Line by Deal Size", {"show_percentage": True}), + ("Countries by Deal Size", {"x_axis_sort": "name"}), + ("Lines by Quantity", {"x_axis_sort": "SUM(sales)"}), + ("Line by Deal Size", {"xscale_interval": -1}), + ("Line by Deal Size", {"left_margin": "auto"}), ("Revenue KPI", {"compare_lag": 3}), ("Orders This Quarter", {"time_range": "Last month"}), ("Revenue and Orders", {"limit_b": 9}), @@ -407,6 +547,7 @@ def test_plan_reports_a_display_change_made_in_the_ui(chart, change, monkeypatch LINE = {"name": "L", "type": "timeseries_line", "dataset": DS, "metrics": ["SUM(x)"], "time_column": "ts"} +BAR = {"name": "B", "type": "bar", "dataset": DS, "x_column": "c", "metrics": ["SUM(x)"]} @pytest.mark.parametrize("chart, message", [ @@ -442,6 +583,11 @@ def test_plan_reports_a_display_change_made_in_the_ui(chart, change, monkeypatch ({"name": "M", "type": "mixed", "dataset": DS, "x_column": "ts", "y_axis_min_secondary": 3, "y_axis_max_secondary": 2, "a": {"metrics": ["SUM(x)"]}, "b": {"metrics": ["SUM(y)"]}}, "y_axis_min_secondary (3) must be below y_axis_max_secondary (2)"), + ({**BAR, "sort_by": "SUM(y)", "category_sort": "asc"}, "set category_sort or sort_by, not both"), + ({**BAR, "sort_by": "total"}, "one series already ranks by its metric"), + ({**BAR, "sort_by": "SUM(y)", "groupby": "g"}, "Superset ranks grouped bars by their series only"), + ({**BAR, "sort_by": "total", "groupby": "g", "contribution": "row"}, + "sort_by ranks by values, and contribution plots shares"), ]) def test_wrong_combinations_are_named(chart, message): with pytest.raises(ValidationError) as err: @@ -481,6 +627,25 @@ def test_series_limit_metric_is_resolved_like_any_metric(): assert [(e.code, e.ref) for e in res.errors] == [("metric_not_found", "no_such_metric")] +def test_a_bars_sort_metric_is_resolved_and_queried_by_smoke(): + """A sort metric the bar doesn't draw is still in its query, so check names it and + the smoke query carries it (a bad one fails the chart, not just its order).""" + from chartwright.smoke import _query_for + + ds = ResolvedDataset(id=1, uuid="u", table="t", schema=None, database_name="examples", + columns=["c", "x"], metrics=["revenue"]) + res = Resolution() + _check_chart_fields(_spec({**BAR, "sort_by": "SUM(no_such_column)"}).charts[0], ds, res) + assert [(e.code, e.ref) for e in res.errors] == [("column_not_found", "no_such_column")] + res = Resolution() + _check_chart_fields(_spec({**BAR, "sort_by": "revenue"}).charts[0], ds, res) + assert res.errors == [] + spec = _spec({**BAR, "sort_by": "revenue"}) + assert _query_for(spec.charts[0], spec)["metrics"][1] == "revenue" + spec = _spec({**BAR, "metrics": ["SUM(x)", "COUNT(*)"], "sort_by": "total"}) + assert len(_query_for(spec.charts[0], spec)["metrics"]) == 2 + + # -- a written Superset default -------------------------------------------------------- PIE = {"name": "P", "type": "pie", "dataset": DS, "metric": "COUNT(*)", "groupby": "g"} diff --git a/tests/test_standards.py b/tests/test_standards.py index dfa7750..6b268a0 100644 --- a/tests/test_standards.py +++ b/tests/test_standards.py @@ -497,7 +497,7 @@ def test_the_standards_disable_list_is_reported_as_ignored(repo, capsys): def test_explain_names_the_standard_chain(repo, capsys): spec = write_spec(repo / "specs" / "s.json", standard="finance") text = run_ok(capsys, "explain", str(spec)) - assert text.splitlines()[0] == ("Design defaults (design brain 7, audience analytical, " + assert text.splitlines()[0] == ("Design defaults (design brain 12, audience analytical, " "standard org -> finance)") payload = run_ok(capsys, "explain", str(spec), "--json") assert payload["standard"] == {"name": "finance", "chain": ["org", "finance"], @@ -889,7 +889,7 @@ def test_no_standards_directory_changes_nothing(tmp_path, capsys): assert all("layer" not in f and "locked" not in f for f in payload["findings"]) assert payload == advise(load_spec(DATA)).payload() text = run_ok(capsys, "explain", str(spec)) - assert text.splitlines()[0] == "Design defaults (design brain 7, audience analytical)" + assert text.splitlines()[0] == "Design defaults (design brain 12, audience analytical)" def _bundle_hash(data: dict) -> str: diff --git a/tests/test_standards_content.py b/tests/test_standards_content.py index a1ccbb3..032944b 100644 --- a/tests/test_standards_content.py +++ b/tests/test_standards_content.py @@ -1447,7 +1447,7 @@ def test_explain_has_a_dashboard_section(repo, capsys): "released", "bnbColors", "supersetColors") assert scheme["override"].startswith("it is yours") text = run_ok(capsys, "explain", str(path)) - assert text.splitlines()[0].startswith("Design defaults (design brain 7") + assert text.splitlines()[0].startswith("Design defaults (design brain 12") assert "Dashboard content (standard org -> finance, via design.standard)" in text # A chart field the standard wrote says so in the chart section too. k = next(c for c in payload["charts"] if c["chart"] == "K") diff --git a/tests/test_superset_version.py b/tests/test_superset_version.py index 14eec95..4c475a5 100644 --- a/tests/test_superset_version.py +++ b/tests/test_superset_version.py @@ -161,6 +161,17 @@ def test_fields_older_releases_ignore_warn_and_never_block(): assert unknown.ok and len(unknown.warnings) == 4 +def test_a_heatmaps_label_step_is_no_later_release_field(): + """A heatmap's x_label_every is its own xscale_interval, which every release reads + (Heatmap/controlPanel.tsx at 4.1.4, 5.0.0 and 6.1.0); only an axis chart's needs 6.1.0.""" + heatmap = {"name": "Heat", "type": "heatmap", "x_column": "region", "y_column": "city", + "metric": "SUM(amount)", "dataset": DS, "x_label_every": 2, "y_label_every": 1, + "left_margin": 16} + assert check_spec_version(spec(charts=[heatmap]), "4.1.4").warnings == [] + out = check_spec_version(spec(charts=[heatmap, {**LINE, "x_label_every": True}]), "4.1.4") + assert [(w["field"], w["chart"]) for w in out.warnings] == [("x_label_every", "Line")] + + def test_stacked_value_labels_warn_on_what_the_author_wrote(): """only_total defaults to true, so the warning names show_value on the stacked query, which the author wrote, and says what the older release draws."""