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
28 changes: 28 additions & 0 deletions .issueflows/03-solved-issues/issue537_original.md
Original file line number Diff line number Diff line change
@@ -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)
38 changes: 38 additions & 0 deletions .issueflows/03-solved-issues/issue537_plan.md
Original file line number Diff line number Diff line change
@@ -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["<name>"]` → `hdr_journal.<name>`. 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.
27 changes: 27 additions & 0 deletions .issueflows/03-solved-issues/issue537_status.md
Original file line number Diff line number Diff line change
@@ -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.
6 changes: 6 additions & 0 deletions HISTORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions cellpy/utils/batch_tools/batch_plotters.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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)


Expand Down
2 changes: 1 addition & 1 deletion cellpy/utils/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading