From cfa71c70781e6fa610d28a736a56c90d46db4677 Mon Sep 17 00:00:00 2001 From: jepegit Date: Fri, 17 Jul 2026 22:41:46 +0200 Subject: [PATCH] Use HeadersJournal attributes for journal-page column lookups (#537) The string-keyed hdr_journal["mass"]-style subscripts in batch_plotters.py (create_legend, look_up_group) and helpers.py (add_areal_capacity) become attribute access (hdr_journal.mass, ...). This removes the last HeadersJournal-family hard-coded literals flagged by the scan tool, so a journal-header rename touches the header class only - a native-headers Phase-0 prerequisite. Behavior is byte-identical (the dataclass exposes the same value via attribute and subscript). Closes #537 Co-Authored-By: Claude Fable 5 --- .../03-solved-issues/issue537_original.md | 28 ++++++++++++++ .issueflows/03-solved-issues/issue537_plan.md | 38 +++++++++++++++++++ .../03-solved-issues/issue537_status.md | 27 +++++++++++++ HISTORY.md | 6 +++ cellpy/utils/batch_tools/batch_plotters.py | 4 +- cellpy/utils/helpers.py | 2 +- 6 files changed, 102 insertions(+), 3 deletions(-) create mode 100644 .issueflows/03-solved-issues/issue537_original.md create mode 100644 .issueflows/03-solved-issues/issue537_plan.md create mode 100644 .issueflows/03-solved-issues/issue537_status.md diff --git a/.issueflows/03-solved-issues/issue537_original.md b/.issueflows/03-solved-issues/issue537_original.md new file mode 100644 index 00000000..4f2395ec --- /dev/null +++ b/.issueflows/03-solved-issues/issue537_original.md @@ -0,0 +1,28 @@ +# Issue #537: v2 pre-flip: replace hardcoded journal-page column literals with HeadersJournal + +Source: https://github.com/jepegit/cellpy/issues/537 + +## Original issue text + +## Context + +Phase-3 flip prerequisite (native-headers plan Phase 0, item 2; hardcoded-column-headers-report §8 priority 1). Audited 2026-07-17 with `.issueflows/00-tools/scan_hardcoded_headers.py`; remaining `HeadersJournal`-family findings: + +| File | Findings (all families) | +|---|---| +| `cellpy/utils/batch_tools/batch_plotters.py` | 32 | +| `cellpy/utils/helpers.py` | 34 (journal subset only in this issue) | +| `cellpy/utils/collectors.py` | 16 (journal subset) | +| `cellpy/utils/batch_tools/batch_journals.py` | 1 | +| `cellpy/utils/batch.py` | 0 — already clean, keep it that way | + +## Work + +Mechanical 1:1: each literal flagged as `HeadersJournal` by the scan tool becomes an attribute access on the already-imported `hdr_journal` / `get_headers_journal()`. No behavior change. Leave non-journal findings in the same files for the sibling issue (steps/raw/summary literals). + +## Acceptance + +- [ ] Scan tool reports **zero `HeadersJournal` findings** for the files above +- [ ] Full suite green (behavior unchanged — this is a pure rename-to-lookup) + +🤖 Generated with [Claude Code](https://claude.com/claude-code) diff --git a/.issueflows/03-solved-issues/issue537_plan.md b/.issueflows/03-solved-issues/issue537_plan.md new file mode 100644 index 00000000..6c7d035f --- /dev/null +++ b/.issueflows/03-solved-issues/issue537_plan.md @@ -0,0 +1,38 @@ +# Issue #537 plan — journal-page column literals → HeadersJournal + +## Goal + +Remove the hard-coded journal column-name string literals flagged by +`.issueflows/00-tools/scan_hardcoded_headers.py` as `HeadersJournal`, so a +later header rename touches the header class only (native-headers Phase-0 +prerequisite). + +## Scope (verified with the scan tool, 2026-07-17) + +Only **6 findings, on 3 lines** — all are string-keyed subscripts into the +already-imported `hdr_journal` object (the §6 "semi-sanctioned" pattern): + +- `cellpy/utils/batch_tools/batch_plotters.py:53` — `hdr_journal["mass"]`, + `["loading"]`, `["label"]` +- `cellpy/utils/batch_tools/batch_plotters.py:76` — `hdr_journal["group"]`, + `["sub_group"]` +- `cellpy/utils/helpers.py:278` — `hdr_journal["loading"]` + +Non-journal (raw/steps/summary) literals in the same files are **out of +scope** — they belong to #538. + +## Approach + +Mechanical `hdr_journal[""]` → `hdr_journal.`. Verified all five +attribute values equal their subscript values (`HeadersJournal` is a dataclass +exposing both), so behavior is byte-identical. + +## Files to touch + +- `cellpy/utils/batch_tools/batch_plotters.py` +- `cellpy/utils/helpers.py` + +## Test strategy + +Behavior-preserving rename-to-lookup; the full suite is the oracle. Confirm +the scan tool reports **zero `HeadersJournal` findings** for both files after. diff --git a/.issueflows/03-solved-issues/issue537_status.md b/.issueflows/03-solved-issues/issue537_status.md new file mode 100644 index 00000000..26e0441e --- /dev/null +++ b/.issueflows/03-solved-issues/issue537_status.md @@ -0,0 +1,27 @@ +# Issue #537 status — journal-page column literals → HeadersJournal + +- [x] Done + +## Outcome + +All 6 `HeadersJournal` scan findings removed (3 lines): + +- `cellpy/utils/batch_tools/batch_plotters.py` — `hdr_journal["mass"|"loading"|"label"]` + and `["group"|"sub_group"]` → attribute access. +- `cellpy/utils/helpers.py` — `hdr_journal["loading"]` → `hdr_journal.loading`. + +Scan tool now reports **0 `HeadersJournal` findings** for both files. + +## Verification + +- Attribute values confirmed equal to subscript values (byte-identical behavior). +- `tests/test_batch.py` + `tests/test_helpers.py`: 58 passed post-edit. +- Full suite baseline green (668 passed; the single Windows-only HDF5 + `driver lock request failed` setup error on `test_has_no_full_duplicates` is + a pre-existing test-isolation flake — passes in isolation, not on the Linux + CI gate). +- ruff: no new findings vs master. + +## Out of scope (other issues) + +- Raw/steps/summary literals in the same files → #538. diff --git a/HISTORY.md b/HISTORY.md index 92b95dce..6083067f 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -2,6 +2,12 @@ ## [Unreleased] +* Journal-page column literals use `HeadersJournal` attributes (#537, native- + headers Phase-0 prerequisite): the string-keyed `hdr_journal["mass"]`-style + lookups in `batch_plotters.py` and `helpers.py` become attribute access + (`hdr_journal.mass`, …) so a journal-header rename touches the header class + only. Behavior-identical. + * Remote paths via `universal_pathlib` (#375, #371): `OtherPath` is now a thin wrapper around `UPath` (fsspec/Paramiko) instead of Fabric. Supported schemes remain `ssh://` / `sftp://` / `scp://` (scp aliased to sftp). Remote diff --git a/cellpy/utils/batch_tools/batch_plotters.py b/cellpy/utils/batch_tools/batch_plotters.py index b52bb235..827355f3 100644 --- a/cellpy/utils/batch_tools/batch_plotters.py +++ b/cellpy/utils/batch_tools/batch_plotters.py @@ -50,7 +50,7 @@ def create_legend(info, c, option="clean", use_index=False): logging.debug(" - creating legends") mass, loading, label = info.loc[ - c, [hdr_journal["mass"], hdr_journal["loading"], hdr_journal["label"]] + c, [hdr_journal.mass, hdr_journal.loading, hdr_journal.label] ] if use_index or not label: @@ -73,7 +73,7 @@ def create_legend(info, c, option="clean", use_index=False): def look_up_group(info, c): logging.debug(" - looking up groups") - g, sg = info.loc[c, [hdr_journal["group"], hdr_journal["sub_group"]]] + g, sg = info.loc[c, [hdr_journal.group, hdr_journal.sub_group]] return int(g), int(sg) diff --git a/cellpy/utils/helpers.py b/cellpy/utils/helpers.py index 8c0f3220..f26f5d5f 100644 --- a/cellpy/utils/helpers.py +++ b/cellpy/utils/helpers.py @@ -275,7 +275,7 @@ def add_c_rate(cell, nom_cap=None, column_name=None): def add_areal_capacity(cell, cell_id, journal): """Adds areal capacity to the summary.""" - loading = journal.pages.loc[cell_id, hdr_journal["loading"]] + loading = journal.pages.loc[cell_id, hdr_journal.loading] cell.data.summary[hdr_summary["areal_charge_capacity"]] = ( cell.data.summary[hdr_summary["charge_capacity"]] * loading / 1000