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
25 changes: 25 additions & 0 deletions .issueflows/03-solved-issues/issue647_original.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Issue #647: Port raw_plot and cycle_info_plot

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

## Original issue text

## Context

Part of epic #567 (Stage 2 — Other plot families on the same skeleton). Plan of record: `architecture-plan/cellpy2-plotting-redesign-plan.md`.

## Scope

Add `prepare/raw.py` and `prepare/steps.py`; route both public functions through the shared backends. Keep `cycle_info_plot`'s matplotlib single-cycle limitation unless expanding it is trivial and oracle-covered. Axis/legend text goes through `units_label()` via `plotting.labels` (extend `labels.py` beyond legend/marker helpers as needed).

## Acceptance

- Oracle cases for both functions × both backends green.
- No hand-composed unit f-strings remain in these two code paths.
- Header lookups use the public schema/header helpers (no new hard-coded legacy names).

## Depends on

#646

Part of epic #567.
115 changes: 115 additions & 0 deletions .issueflows/03-solved-issues/issue647_plan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
# Issue #647 — Plan: port `raw_plot` and `cycle_info_plot` to prepare→spec→render

## Goal

Route public `raw_plot` and `cycle_info_plot` through **context → registry → prepare → backend.render**: add `prepare/raw.py` and `prepare/steps.py`, register two families, move private plotly/matplotlib layout into shared backends (`kind` branches), and replace hand-composed unit f-strings with `units_label()` helpers via `plotting.labels`. Oracle green for both functions × both backends. Keep `cycle_info_plot` matplotlib single-cycle asymmetry.

## Constraints

- Plan of record: [`architecture-plan/cellpy2-plotting-redesign-plan.md`](../../../architecture-plan/cellpy2-plotting-redesign-plan.md) Phase 3; epic [`.issueflows/05-epics/epic567_plan.md`](../05-epics/epic567_plan.md) Stage 2 issue 2 (Published: #647).
- Depends on #646 (merged): `prepare/curves.py`, `spec.extras["kind"]` dispatch, `backend=` / `interactive=` warn_once pattern.
- Design notes: [`plotting-prepare.md`](../04-designs-and-guides/plotting-prepare.md), [`plotting-backends.md`](../04-designs-and-guides/plotting-backends.md), [`plotting-registry.md`](../04-designs-and-guides/plotting-registry.md).
- Epic already decided: **do not** expand matplotlib `cycle_info_plot` to multi-cycle unless trivial + oracle-covered (default: keep asymmetry).
- Oracle gate: `tests/test_figure_specs.py` cases `raw_plot[*]` and `cycle_info_plot[*]` green; prefer **no** intentional `figure_specs.json` regen.
- Scope: these two entry points only — not ICA/DVA (#648), not collectors.
- Acceptance: no new hard-coded legacy header names; no hand-composed unit f-strings in these two code paths.

### Prior art

- `raw_plot` — `cellpy/utils/plotutils.py` (~1445–1712): copy `cell.data.raw`, resolve columns via `_LiveHeaders(cell, "raw")`, build y-lists from `plot_type` / explicit `y`, optional time-unit x column via pint `Q`, then **inline** plotly (`px.line` / `make_subplots`) or matplotlib (single / twin / nrows). Labels today: `f"Voltage ({cell.data.raw_units.voltage})"` etc. Public switch still `interactive=` (bool), not `backend=`.
- `cycle_info_plot` + `_cycle_info_plot_plotly` / `_cycle_info_plot_matplotlib` / `_get_info` / `_plot_step` — same module (~1715–2055). Merge scaled raw + step-table deltas; plotly multi-cycle hover; matplotlib single-cycle twin + step spans. Headers already `_LiveHeaders` (Phase 0); matplotlib still warns on multi-cycle.
- `#646` mirror — `prepare/curves.py` + `registry` family `"cycles"` + `extras["kind"]=="cycles"` in `PlotlyBackend` / `MatplotlibBackend` + thin public orchestrator in `plotutils`. **Mirror this shape twice** (raw + cycle_info).
- `_LiveHeaders` — `plotutils.py` (~551+): schema-backed per-cell header resolution. Required for both prepares; do not reintroduce module-level `get_headers_*()` singletons.
- `units_label` — `cellpy/units.py`; already used in summary/curves prepare. Issue asks extension of `cellpy/plotting/labels.py` beyond legend/marker helpers.
- `CellContext` — `cellpy/plotting/context.py`: summary-oriented today; prepares can reach `ctx.cell` (same as curves) rather than expanding context in this issue.
- Oracle: `tests/figure_spec_support.py` `_other_family_cases()` — both functions × both backends (still `interactive=`).
- Smoke: `tests/test_plotutils_headers.py` (`test_raw_plot_*`, `test_cycle_info_plot_*`).
- Toolbox: scanners unrelated. Graphify present but no need to query beyond known plotting communities — prior art from #646 + plotutils is sufficient.

## Approach

1. **Register two families** in `cellpy/plotting/registry.py`
- `"raw"` — `extras={"entry_point": "raw_plot", "kind": "raw"}`.
- `"cycle_info"` — `extras={"entry_point": "cycle_info_plot", "kind": "cycle_info"}`.
- `column_builder` can list the columns the prepare actually uses (raw: voltage/current/… stems; cycle_info: time/voltage/current + step stats). Keep them out of the summary oracle menu (same pattern as `"cycles"` / `families(entry_point=...)`).

2. **`cellpy/plotting/prepare/raw.py`**
- `RawPrepareConfig` mirrors public `raw_plot` args (`y`, `y_label`, `x`, `x_label`, `title`, `plot_type`, `double_y`, size knobs, backend, `additional_kwargs`).
- `prepare(ctx, family, config) -> (frame, FigureSpec)`:
- Resolve headers via `_LiveHeaders` (import from plotutils **or** move helper into `cellpy.plotting.headers` if the import cycle is ugly — prefer move only if needed).
- Build the plotting frame (including synthetic `test_time_hrs` / days / years columns).
- Emit `FigureSpec` with panels/axes describing the subplot layout (1 row, twin-y, or N rows); put render knobs in `extras` (`kind="raw"`, `y` column list, labels, `double_y`, height heuristics, `backend`, kwargs).
- Axis labels: go through new helpers in `plotting.labels` (see step 4) — no `f"... ({raw_units...})"` in prepare or backends.

3. **`cellpy/plotting/prepare/steps.py`** (cycle-info family)
- `CycleInfoPrepareConfig`: `cycle`, `t_unit` / `v_unit` / `i_unit`, `get_axes`, title/size knobs, backend, kwargs.
- Prepare: compute scalers (`unit_scaler_from_raw`), filter/merge raw+steps (same columns as today), attach scaled series on the returned frame; stash step-annotation inputs / hover customdata columns in `extras` as needed so backends do not re-query the cell.
- Matplotlib single-cycle clamp stays in prepare **or** the mpl branch (warn + take first) — behaviour unchanged.
- `FigureSpec.extras["kind"] = "cycle_info"`.

4. **`plotting.labels` axis helpers**
- Add small formatters, e.g. `quantity_label(name, unit)` → `"Voltage (V)"`, and/or thin wrappers that call `units_label(physical_property, ..., units=...)` then compose the display name.
- **Raw path:** prefer labelling from the cell’s **raw** unit spec when that is what the series still carries (today’s behaviour); if `units_label` only accepts `CellpyUnits`, pass `cell.raw_units` when compatible, else format via the helper using the same unit strings the frame was scaled with (cycle_info already has explicit `t_unit`/`v_unit`/`i_unit`).
- Goal: zero hand-rolled `f"{name} ({unit})"` left in the two public paths / their private helpers.

5. **Backend branches**
- In `PlotlyBackend.render` / `MatplotlibBackend.render`:
- `kind == "raw"` → port current inline raw layout.
- `kind == "cycle_info"` → port `_cycle_info_plot_*` (plus keep `_get_info` / `_plot_step` as private backend or prepare helpers).
- else existing summary / cycles paths.
- Mechanical port first (oracle parity). Delete the live private forks from `plotutils` once unused.

6. **Thin public entry points** in `cellpy/utils/plotutils.py`
- Same orchestration as `cycles_plot`: resolve `backend=` vs deprecated `interactive=` (`warn_once`, removal 2.1) → `from_source` → `registry.get(...)` → prepare → `get_backend(...).render`.
- Preserve `raw_plot` return figure; preserve `cycle_info_plot` `get_axes` / `fig.show()` / matplotlib axes semantics (including plotly default show + `None` return that the oracle already documents).
- Register deprecations in `_deprecation` seed + regen `DEPRECATIONS.md`.

7. **Tests / oracle harness**
- Point `raw_plot[*]` / `cycle_info_plot[*]` cases at `backend=` (drop `interactive=` noise).
- New focused tests (names flexible): prepare returns `(frame, FigureSpec)` with correct `kind`; `interactive=` warns+maps; no private `_cycle_info_plot_*` left on the public path; header smoke tests still pass.
- Prefer no snapshot regen; if axis-title text changes solely because of `units_label` wording, regenerate in the same commit and note it in the PR.

8. **Design notes**
- Update `plotting-prepare.md` (raw + steps) and `plotting-backends.md` (`kind` raw / cycle_info). Brief registry note for the two families.

## Files to touch

| Path | Change |
|---|---|
| `cellpy/plotting/prepare/raw.py` | **new** — raw frame + `FigureSpec` |
| `cellpy/plotting/prepare/steps.py` | **new** — cycle-info merge/scale + `FigureSpec` |
| `cellpy/plotting/prepare/__init__.py` | export / note |
| `cellpy/plotting/registry.py` | register `"raw"` and `"cycle_info"` |
| `cellpy/plotting/labels.py` | axis/quantity label helpers via `units_label` |
| `cellpy/plotting/backends/plotly.py` | `kind` branches for raw + cycle_info |
| `cellpy/plotting/backends/mpl.py` | same |
| `cellpy/utils/plotutils.py` | thin `raw_plot` / `cycle_info_plot`; delete private layout helpers |
| `cellpy/plotting/headers.py` (optional) | move `_LiveHeaders` only if import cycle forces it |
| `cellpy/_deprecation.py` + `DEPRECATIONS.md` | `interactive=` for both entry points |
| `tests/figure_spec_support.py` | cases → `backend=` |
| `tests/test_raw_prepare.py` / `tests/test_cycle_info_prepare.py` (flexible) | **new** — prepare / deprecations / no private fork |
| `tests/test_plotutils_headers.py` | keep green; adjust kwargs if needed |
| `tests/test_figure_specs.py` | run; regen snapshot only if titles intentionally change |
| `.issueflows/04-designs-and-guides/plotting-*.md` | document the two prepares + kinds |

## Test strategy

```bash
uv sync --extra batch
MPLBACKEND=Agg uv run pytest tests/test_raw_prepare.py tests/test_cycle_info_prepare.py tests/test_plotutils_headers.py tests/test_figure_specs.py -q
MPLBACKEND=Agg uv run pytest -m essential --ignore=tests/test_arbin_variants_two_stage.py
```

Parity focus: default oracle cell × both backends for both functions; `plot_type` smoke for raw; cycle=3 for cycle_info; matplotlib multi-cycle warn + single-cycle behaviour unchanged.

## Open questions

1. **`kind` / registry names** — **Recommend:** `"raw"` / `"cycle_info"` (matches entry points; issue’s `prepare/steps.py` is the module name, not the kind string). Alternative: `"steps"` as kind — slightly clearer file↔kind link, worse API symmetry.
2. **Flip `backend=` / deprecate `interactive=` on both?** — **Recommend:** yes (global epic policy; matches #639/#646; oracle harness update is cheap).
3. **Where `_LiveHeaders` lives** — **Recommend:** keep in `plotutils` and import into prepare unless that creates a cycle; then move to `cellpy/plotting/headers.py`. Do not duplicate.
4. **Raw axis units source** — **Recommend:** label with the unit string that matches the plotted series (today: `raw_units` via a labels helper). Do not silently switch to `cellpy_units` if that changes oracle axis titles. Alternative: always `cellpy_units` — only if we accept snapshot churn.
5. **Matplotlib multi-cycle for `cycle_info_plot`?** — **Recommend:** keep single-cycle asymmetry (epic default). Expanding is a follow-up.

## Scope check

One Stage-2 slice: two prepares + two registry families + two backend kinds + thin public APIs + label helpers + deprecations. Fits a single PR. Follow-up already published: #648 (ICA/DVA).
18 changes: 18 additions & 0 deletions .issueflows/03-solved-issues/issue647_status.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Issue #647 — Status

- [x] Done

## What's done

- Plan accepted; implemented on `647-port-raw-and-cycle-info-plot`.
- Moved `LiveHeaders` to `cellpy/plotting/headers.py` (re-exported as `plotutils._LiveHeaders`).
- Extended `plotting.labels` with `quantity_label` / `units_quantity_label`.
- Added `prepare/raw.py` + `prepare/steps.py`; registered `"raw"` / `"cycle_info"`.
- Backend `kind` branches for raw + cycle_info; thin public APIs + `interactive=` deprecations.
- Tests (`test_raw_cycle_info_prepare.py`), oracle harness `backend=`, design notes updated.
- Focused suite + essential gate green (oracle unchanged — no snapshot regen).
- HISTORY.md Unreleased bullet; closed via `/iflow-close`.

## Remaining work

- None (PR merge + `/iflow-cleanup` after merge).
20 changes: 12 additions & 8 deletions .issueflows/04-designs-and-guides/plotting-backends.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@

Epic #567 Stage 1 needs one layout engine and prepare→spec→render for
`summary_plot`, with two public backends (`plotly` | `matplotlib`). Stage 2
(#646) adds a second render branch for `cycles_plot`.
adds further render branches for `cycles_plot` (#646) and `raw_plot` /
`cycle_info_plot` (#647).

## Decision

Expand All @@ -16,14 +17,17 @@ Epic #567 Stage 1 needs one layout engine and prepare→spec→render for
- **Static path:** `MatplotlibBackend.render(frame, spec)` (#639). Seaborn is
used only for palette/style/faceting helpers (`relplot`); it is **not** a
public backend name. `SeabornPlotBuilder` is deleted.
- **Public switch:** `summary_plot(..., backend="plotly"|"matplotlib")` and
`cycles_plot(..., backend=...)`. `interactive=` is a `warn_once` alias
(removal 2.1) on both.
- **Family dispatch (#646):** when `spec.extras.get("kind") == "cycles"`,
both backends take the voltage–capacity render path (ported from the old
private `_cycles_plotter_*` helpers). Otherwise they keep the summary path.
- **Public switch:** `summary_plot(..., backend="plotly"|"matplotlib")`,
`cycles_plot(..., backend=...)`, `raw_plot(..., backend=...)`,
`cycle_info_plot(..., backend=...)`. `interactive=` is a `warn_once` alias
(removal 2.1) on all of them.
- **Family dispatch:** `spec.extras.get("kind")` selects the render branch:
- `"cycles"` — voltage–capacity (#646)
- `"raw"` — raw time-series (#647)
- `"cycle_info"` — raw + step annotations (#647; matplotlib single-cycle)
- otherwise — summary path

## Links

- Issues #646, #639, #638, #637, #636; epic #567
- Issues #647, #646, #639, #638, #637, #636; epic #567
- Plan of record: `architecture-plan/cellpy2-plotting-redesign-plan.md` §3.1
23 changes: 17 additions & 6 deletions .issueflows/04-designs-and-guides/plotting-prepare.md
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
# Plotting prepare (summary + curves)
# Plotting prepare (summary + curves + raw + cycle_info)

## Context

`SummaryPlotDataPreparer` lived in `cellpy/utils/plotutils.py` and fed both
plotly and seaborn builders. Epic #567 Stage 1 needs prepare → `FigureSpec` →
backend.render as the only summary path. Stage 2 (#646) extends the same
contract to `cycles_plot`.
backend.render as the only summary path. Stage 2 extends the same contract to
`cycles_plot` (#646), then `raw_plot` / `cycle_info_plot` (#647).

## Decision

Expand All @@ -22,12 +22,23 @@ contract to `cycles_plot`.
form/rest frames and styling knobs. Curve load seam defaults to
`c.get_cap` (oracle-stable); `cellpycore.curves` preferred path can land
later behind `_load_curve_frame`.
- **Raw prepare** lives in `cellpy/plotting/prepare/raw.py` (#647).
`spec.extras["kind"] == "raw"`; predefined `plot_type` menus and synthetic
time columns are resolved here. Axis labels use `plotting.labels`
(`units_quantity_label` / `quantity_label`) against `raw_units`.
- **Cycle-info prepare** lives in `cellpy/plotting/prepare/steps.py` (#647).
`spec.extras["kind"] == "cycle_info"`. Plotly path emits a merged scaled
frame; matplotlib keeps the single-cycle asymmetry and stashes the step
table on `extras["steps"]`.
- **`LiveHeaders`** lives in `cellpy/plotting/headers.py` (re-exported as
`plotutils._LiveHeaders`).
- **`CellContext`** in `cellpy/plotting/context.py` is the thin cell adapter;
BatchContext waits for collectors rebase.
- Public `summary_plot` / `cycles_plot` stay in `plotutils` and orchestrate
context → registry → prepare → `get_backend(backend).render`.
- Public `summary_plot` / `cycles_plot` / `raw_plot` / `cycle_info_plot` stay
in `plotutils` and orchestrate context → registry → prepare →
`get_backend(backend).render`.

## Links

- Issues #646, #639, #638; epic #567
- Issues #647, #646, #639, #638; epic #567
- Related: `plotting-registry.md`, `plotting-backends.md`
6 changes: 3 additions & 3 deletions .issueflows/04-designs-and-guides/plotting-registry.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ moves selection into `cellpy.plotting.registry`.
backend consumption is #639.
- **Oracle menu** (`tests/figure_spec_support.SUMMARY_FAMILIES`) derives from
`families(entry_point="summary_plot")` so the snapshot menu cannot drift
from the runtime menu. Non-summary families (e.g. `"cycles"` for
`cycles_plot`, #646) register with `extras={"entry_point": "cycles_plot"}`
and are excluded from the summary oracle.
from the runtime menu. Non-summary families register with
`extras={"entry_point": "..."}` and are excluded from the summary oracle:
`"cycles"` (`cycles_plot`, #646), `"raw"` / `"cycle_info"` (#647).

## Alternatives considered

Expand Down
2 changes: 2 additions & 0 deletions DEPRECATIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ uv run python -m cellpy._deprecation

| Name | Replacement | Introduced | Removal |
| --- | --- | --- | --- |
| `cycle_info_plot(interactive=...)` | `backend="plotly"|"matplotlib"` | 2.0 | 2.1 |
| `cycles_plot(interactive=...)` | `backend="plotly"|"matplotlib"` | 2.0 | 2.1 |
| `cycles_plot(xlim=...)` | `cycles_plot(x_range=...)` | 2.0 | 2.1 |
| `cycles_plot(ylim=...)` | `cycles_plot(y_range=...)` | 2.0 | 2.1 |
Expand All @@ -21,5 +22,6 @@ uv run python -m cellpy._deprecation
| `legacy header attribute access (headers_normal / _summary / _step_table)` | `c.schema.raw / c.schema.steps / c.schema.summary` | 2.0 | 2.1 |
| `make_new_cell` | `CellpyCell.vacant` | 2.0 | 2.1 |
| `plotutils.summary_plot_legacy` | `cellpy.utils.plotutils.summary_plot (same figures, same options)` | 2.0 | 2.1 |
| `raw_plot(interactive=...)` | `backend="plotly"|"matplotlib"` | 2.0 | 2.1 |
| `summary_plot(interactive=...)` | `backend="plotly"|"matplotlib"` | 2.0 | 2.1 |
| `the 'dq' column of the ica output frame` | `the 'dqdv' column of the same frame` | 2.0 | 2.1 |
2 changes: 2 additions & 0 deletions HISTORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## [Unreleased]

* Port raw_plot and cycle_info_plot to prepare→spec→render (#647).

* Port cycles_plot to prepare→spec→render (#646).

* Matplotlib backend; retire SeabornPlotBuilder; unify backend= (#639).
Expand Down
11 changes: 11 additions & 0 deletions cellpy/_deprecation.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,17 @@ def _seed_known_deprecations() -> None:
"cycles_plot(y_range=...)",
removal="2.1",
)
# Stage 2 (#647): raw_plot / cycle_info_plot backend=.
_register(
"raw_plot(interactive=...)",
'backend="plotly"|"matplotlib"',
removal="2.1",
)
_register(
"cycle_info_plot(interactive=...)",
'backend="plotly"|"matplotlib"',
removal="2.1",
)


if __name__ == "__main__":
Expand Down
9 changes: 8 additions & 1 deletion cellpy/plotting/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,12 @@
make_matplotlib_manager,
save_matplotlib_figure,
)
from cellpy.plotting.labels import legend_replacer, remove_markers
from cellpy.plotting.labels import (
legend_replacer,
quantity_label,
remove_markers,
units_quantity_label,
)
from cellpy.plotting.prepare import prepare_summary
from cellpy.plotting.registry import (
PlotFamily,
Expand Down Expand Up @@ -70,6 +75,8 @@
"make_matplotlib_manager",
"make_plotly_template",
"prepare_summary",
"quantity_label",
"remove_markers",
"save_matplotlib_figure",
"units_quantity_label",
]
Loading
Loading