Skip to content

Fix cellpycore 0.2.2 sync fallout: merger summary test_id + stale tests#524

Merged
jepegit merged 1 commit into
masterfrom
fix/cellpycore-022-sync
Jul 17, 2026
Merged

Fix cellpycore 0.2.2 sync fallout: merger summary test_id + stale tests#524
jepegit merged 1 commit into
masterfrom
fix/cellpycore-022-sync

Conversation

@jepegit

@jepegit jepegit commented Jul 17, 2026

Copy link
Copy Markdown
Owner

Summary

cellpycore 0.2.2 (core #136) bridges test_id through the legacy steps/summary output. The Milestone C sync (#523) regenerated goldens but left four non-essential tests failing on master and exposed a real merger bug.

  • merger fix: campaign_fold concatenated the right-hand summary without remapping its test_id, so a merged (pre-recompute) campaign summary carried test_id=0 on every row — and would round-trip wrongly through v9. The right summary is now remapped via the merge id-map, mirroring the raw-frame handling.
  • cellreader cleanup: removed the v2: composite group keys + multi-file merge (V2-03/07) #507 re-stamp workaround in make_step_table — core 0.2.2 delivers test_id on the bridge output directly, which was the point of core Make the C-rate available in the summary file #136.
  • stale tests updated: the two test_save_cellpyfile_auto_extension* tests now expect the v9 .cellpy default (v9 cellpy-file format (zip-of-parquet) — #510 Milestone A #521); test_single_test_steps_have_no_test_id_column flipped to assert the bridged test_id=0.
  • new regression test: test_campaign_merge_remaps_summary_test_id.

Test plan

  • Full suite: 643 passed, 0 failed (was 4 failed on master)
  • cellpy-core 0.2.2 suite: 261 passed
  • ruff: no new findings vs master

🤖 Generated with Claude Code

Core #136 bridges test_id through the legacy steps/summary output, which
left four non-essential tests failing on master and exposed a merger bug:
campaign_fold concatenated the right-hand summary without remapping its
test_id, so a merged (pre-recompute) summary carried test_id=0 on every
row and would round-trip wrongly through v9.

- merger: remap the right summary's test_id via the merge id-map
  (mirrors the raw handling)
- cellreader: drop the #507 re-stamp workaround in make_step_table
  (core 0.2.2 delivers test_id directly)
- tests: update the .h5 -> .cellpy default-extension expectations and the
  single-test no-test_id assertion; add a regression test for the
  merger remap

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cursor

cursor Bot commented Jul 17, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@jepegit
jepegit merged commit 39ab68a into master Jul 17, 2026
4 checks passed
@jepegit
jepegit deleted the fix/cellpycore-022-sync branch July 17, 2026 13:48
jepegit added a commit that referenced this pull request Jul 17, 2026
…) (#532)

The essential-only gate let four non-essential test failures land on
master (fixed in #524). The full linux suite runs in ~3 min, so the
marker split no longer buys much on linux: add a full-suite job to the
tier-1 workflow, mirroring the essential job's setup and the tier-3
linux ignore (plotutils summary plot needs a display).

Making it required is a branch-protection (admin) flip once this lands.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
jepegit added a commit that referenced this pull request Jul 18, 2026
On a native cell, merge()/campaign_fold now works: translate self.data and copies
of the participants to legacy names (keeping test_id on every frame so the merge's
per-source id assignments survive), run the existing legacy merge machinery
(_append / campaign_fold, incl. the #524 test_id remap + renumber_cycles), then
translate the result back to native. Copies keep the source cells' native frames
intact.

Drops the NotImplementedError; replaces test_native_merge_raises with native
campaign + continuation merge round-trip tests. The legacy test_merge_campaign
suite stays pinned to native_schema=False as it validates the core machinery the
native path round-trips through. Full suite: 677 passed.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant