Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .issueflows/03-solved-issues/issue801_original.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# Issue #801: App-friendly collected figures: theme / label / height hook (or pass a FigureSpec)

Source: https://github.com/jepegit/cellpy/issues/801

## Original issue text

Split from #791 (item c). `collected_plot` returns faceted figures with default plotly styling (mirror axis boxes, right-side facet titles spelled `variable=charge_capacity_gravimetric`, auto height growing with facet count). A **theme / label / height hook** (or a `FigureSpec` the caller can pass) would let apps drop the figure in without re-styling every one.

Relates to the plotting `FigureSpec` pipeline and SPEED-30 label work.
98 changes: 98 additions & 0 deletions .issueflows/03-solved-issues/issue801_plan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
# Issue #801 — Plan: App-friendly collected figures (theme / label / height)

Status: **confirmed** (2026-07-31) — open questions resolved as recommended:
pretty labels default; no `spec=` this PR; include `layout_updates`.

## Goal

Give apps first-class, documented knobs on `collected_plot` / `Collection.plot` so faceted Plotly figures can be dropped into an app shell without a private restyle pass — covering **template/theme**, **facet labels**, and **height**.

## Constraints

- Patch release (`v.2.1.2`); stay on the **collected** Plotly path (`Collection.plot` → `collected_plot` → `summary_plotter` / `_cycles_plotter`). Do not rework single-cell prepare→render.
- Collected path still builds a thin `FigureSpec` (extras only; panels unused) — same as #804. Do **not** force a full PanelSpec migration.
- Prefer additive kwargs + clearer defaults over a new theming framework.
- Companion context: [cellpy/cellpy-simple-gui](https://github.com/cellpy/cellpy-simple-gui) pain-point §11 (restyles every figure today); #804 already landed `share_y` / `y_ranges`.
- Design docs: [plotting-collected.md](../04-designs-and-guides/plotting-collected.md), [plotting-backends.md](../04-designs-and-guides/plotting-backends.md).

### Prior art

| Hit | Where | Relation |
| --- | --- | --- |
| Hardcoded `template = f"{PLOTLY_BASE_TEMPLATE}+{method}"` | [`collected.py`](../../cellpy/plotting/collected.py) `_cycles_plotter` | Theme seam — no public override today |
| `make_collector_templates` / `make_plotly_template` | [`theme.py`](../../cellpy/plotting/theme.py) | Axis chrome only; apps still restyle paper/legend/facet strip |
| `y_label_mapper` + `_plotly_y_label_cleaner` | `sequence_plotter` / `summary_plotter` | Label seam exists; only populated when `units=` is passed (Batch path). Plain `Collection.plot` keeps `variable=…` facet titles |
| `height` / `sub_fig_min_height` / `figure_border_height` | `_cycles_plotter` | Height seam exists but names are obscure; summary forces `sub_fig_min_height=300` |
| `plotly_template` on single-cell prepare configs | `prepare/{summary,curves,ica}.py` | Naming to mirror for collected override |
| Thin `FigureSpec(extras=…)` in `collected_plot` | [`collected.py`](../../cellpy/plotting/collected.py) | Escape hatch possible via merging `render_opts`; panels still ignored |
| Toolbox / graphify | `.issueflows/00-tools/`, `graphify-out/` | No plotting-theme helper |

## Approach

Ship **three public knobs** (kwargs through `Collection.plot` / `collected_plot` → `render_opts`), document them, and improve the default label path so apps are not forced to pass `units=` just to drop `variable=`.

### 1. Theme / template

- Accept `plotly_template=` (string; Plotly template name or `"+"`-combined). When set, use it in `_cycles_plotter`'s `fig.update_layout(template=…)` instead of hardcoding `simple_white+{method}`.
- Accept optional `layout_updates: Mapping[str, Any]` applied after the collector layout (paper/plot bgcolor, margin, legend dict patches). Keep this a shallow `update_layout(**layout_updates)` — not a second theme system.
- Out of scope: new light/dark token packs, discrete colorway registry (existing `palette=` / `palette_*` stay as-is).

### 2. Labels (facet strip / y titles)

- When `y_label_mapper` is omitted, build a **default pretty mapper** from the frame's `variable` values (title-case / split `_`, optional unit suffix only if `units=` present — reuse the existing units branch). Always strip Plotly's `variable=` annotation text (move facet label onto the y-axis title and clear the side strip), matching today's `y_label_mapper` behaviour.
- Keep explicit `y_label_mapper=` as override (wins over the default).
- Gate the new default behind `pretty_labels=True` **if** Open question 1 chooses opt-in; otherwise make it the summary default (recommended).

### 3. Height

- Document and prefer public names:
- `height=` — absolute figure height (already works).
- `height_per_panel=` — alias of `sub_fig_min_height` (clearer for apps).
- keep `figure_border_height=` / `sub_fig_min_height=` as synonyms.
- Ensure summary's internal default (`300` per panel) still applies when neither absolute `height` nor `height_per_panel` is given.
- No change to the experimental `height_fractions` path.

### 4. Optional `spec=` (thin)

- Allow `collected_plot(..., spec=FigureSpec)` / `spec=` on `Collection.plot`: merge `spec.extras["render_opts"]` into opts, honour `spec.title`, and if `spec.extras` carries `plotly_template` / `layout_updates`, treat them like the kwargs above. Still ignore `panels` (same as #804).
- If Open question 2 says kwargs-only, skip this bullet.

### 5. Docs

- Update `plotting-collected.md` with the three knobs + a short app example.
- Docstrings on `collected_plot` / `Collection.plot` / `summary_plotter`.
- Tiny example in existing plotting/collect docs if a natural home exists (no new guide).

## Files to touch

| Path | Change |
| --- | --- |
| [`cellpy/plotting/collected.py`](../../cellpy/plotting/collected.py) | `plotly_template`, `layout_updates`, default/`pretty_labels` mapper, `height_per_panel` alias; optional `spec=` merge; docstrings |
| [`cellpy/collect/collection.py`](../../cellpy/collect/collection.py) | Docstring mention of the new knobs |
| [`.issueflows/04-designs-and-guides/plotting-collected.md`](../04-designs-and-guides/plotting-collected.md) | Decision bullets + example |
| `tests/test_collected_summary_axes.py` or new `tests/test_collected_app_hooks.py` | Assert template override, pretty labels (no `variable=`), height math |

## Test strategy

```bash
uv run pytest tests/test_collected_summary_axes.py tests/test_collected_app_hooks.py -q
MPLBACKEND=Agg uv run pytest -m essential
```

- Synthetic long summary frame (reuse #804 helper pattern).
- Assert: with `plotly_template="plotly_white"`, layout template reflects it; with default/pretty labels, annotations empty or lack `variable=`; with `height_per_panel=180` and 2 variables, `layout.height == figure_border_height + 2*180` (or documented formula).
- Mark essential only if cheap (same bar as #804).

## Open questions

1. **Pretty labels default?**
- **Recommended: yes** — summary collected path always builds a pretty mapper when `y_label_mapper` is omitted (visual change: side `variable=…` strip goes away; y-axis titles become humanized).
- Alternative: opt-in `pretty_labels=True`.

2. **`spec=` escape hatch this PR?**
- **Recommended: no** — kwargs cover the issue text; thin `FigureSpec` merge can wait.
- Alternative: accept `spec=` merge as above.

3. **`layout_updates=` this PR?**
- **Recommended: yes** (small; unblocks paper/plot colors without a template factory).
- Alternative: template string only.
21 changes: 21 additions & 0 deletions .issueflows/03-solved-issues/issue801_status.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# Issue #801 — Status

- [x] Done

## What's done

- Plan accepted (pretty labels default; no `spec=`; include `layout_updates`).
- Implemented on collected Plotly path:
- `plotly_template=` override
- `layout_updates=` after collector layout
- default pretty `y_label_mapper` (clears `variable=…` facet strip)
- `height_per_panel=` alias of `sub_fig_min_height`
- `y_ranges` applied before label cleanup (keeps #804 working with pretty labels)
- Docs: `plotting-collected.md`, docstrings on `collected_plot` / `Collection.plot` / `summary_plotter`.
- Tests: `tests/test_collected_app_hooks.py` + updated `tests/test_collected_summary_axes.py`.
- `HISTORY.md` Unreleased bullet added.
- Essential suite green (`635 passed`); PR https://github.com/jepegit/cellpy/pull/808

## Remaining work

- None — ready for merge / `/iflow-cleanup` after merge.
20 changes: 20 additions & 0 deletions .issueflows/04-designs-and-guides/plotting-collected.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,13 @@ Epic #567 Stage 3 / issue #657 re-bases collectors' drawing half onto
`y_ranges={"coulombic_efficiency": [0, 110], ...}` (variable → `[lo, hi]`).
Non-empty `y_ranges` forces independent axes. Plotly is the supported
backend for `y_ranges`; seaborn/matplotlib are best-effort and ignore it.
- **App chrome (#801):** `plotly_template=` overrides the default
`plotly+{method}` combo; `layout_updates=` is a shallow
`fig.update_layout(**…)` after collector styling. Summary facet strips no
longer keep raw `variable=…` text by default — pretty y-axis titles are
built automatically (`y_label_mapper=` overrides). Height:
`height=` (absolute) or `height_per_panel=` (alias of `sub_fig_min_height`;
summary default 300 px/panel) plus optional `figure_border_height=`.

### Example — Capacity + CE without crushing panels

Expand All @@ -45,9 +52,22 @@ collection.plot(
)
```

### Example — drop into an app shell

```python
collection.plot(
family_kind="summary",
plotly_template="plotly_white",
layout_updates={"paper_bgcolor": "#f7f7f7", "margin": dict(l=60, r=20, t=40, b=40)},
height_per_panel=220,
y_ranges={"coulombic_efficiency": [0, 110]},
)
```

## Links

- Issue #657; epic #567; plan
`architecture-plan/cellpy2-plotting-redesign-plan.md` §3.3 / Phase 4
- Related: `plotting-prepare.md`, `plotting-backends.md`
- Issue #804 (per-panel y-limits / `share_y`)
- Issue #801 (theme / label / height hooks)
3 changes: 3 additions & 0 deletions HISTORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## [Unreleased]

* App-friendly collected figures: theme / label / height hook (or pass a
FigureSpec). (#801)

## [2.1.1.post3] - 2026-07-30

Post-release of 2.1.1 — silence loader-discovery WARNING spam for apps.
Expand Down
4 changes: 3 additions & 1 deletion cellpy/collect/collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,9 @@ def plot(self, *, family_kind: str | None = None, **kwargs):

For summary collections (Plotly), pass ``share_y`` / ``match_axes`` and
optional ``y_ranges={variable: [lo, hi], ...}`` for per-facet y-limits
(see :func:`cellpy.plotting.collected.summary_plotter`).
(see :func:`cellpy.plotting.collected.summary_plotter`). App chrome:
``plotly_template``, ``layout_updates``, ``y_label_mapper``,
``height`` / ``height_per_panel``.
"""
from cellpy.plotting import collected_plot

Expand Down
Loading
Loading