Backport #310 to maint/v3: label by= strata from the fit

[ehrlinger]

Backports #310, which Copilot raised in review on #308. #308 shipped the strata fix from #305 to maint/v3 without this follow-up, so 3.5.4 as it stands errors when a subset passed through ... drops a whole by group.

What it carries

Both #310 commits, cherry-picked with -x:

• Fit on a factor bound to grp, so survfit() names each stratum grp=<level>, and read the labels back from those names. A subset or start.time that drops a group now labels the groups that are left; CRAN 3.5.3 relabelled them as the first groups in data.
• When subset leaves a single group (the one case survfit() does not name), the group comes from the rows the fit used, with the subscript applied to the row numbers so negative indices work.

Differences from main

• NEWS.md: the note goes into the existing 3.5.4 strata bullet, worded against 3.5.3 rather than the v4 development line. It also rewraps an overlong line Backport #303, #304, #299 fixes to maint/v3 (3.5.4) #308 introduced.
• No version bump: 3.5.4 is unreleased.

Verification

• lintr::lint_package(): 0.
• NOT_CRAN=true VDIFFR_RUN_TESTS=true devtools::test(): 0 failures, 1578 passed, 5 skipped (existing skips). No snapshot files touched.
• R CMD check --as-cran with the manual, from a git archive export: 1 NOTE (CRAN incoming feasibility).

🤖 Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 85.71429% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.12%. Comparing base (749bb96) to head (0cbe923).
⚠️ Report is 1 commits behind head on maint/v3.

Files with missing lines	Patch %	Lines
R/utils.R	79.16%	5 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##           maint/v3     #311      +/-   ##
============================================
- Coverage     89.13%   89.12%   -0.01%     
============================================
  Files            50       50              
  Lines          4667     4682      +15     
============================================
+ Hits           4160     4173      +13     
- Misses          507      509       +2     

Files with missing lines	Coverage Δ
R/kaplan.R	100.00% <100.00%> (ø)
R/nelson.R	96.96% <100.00%> (+0.04%)	⬆️
R/utils.R	86.11% <79.16%> (-1.39%)	⬇️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Copilot review overview

🟡 Changes recommended

subset expressions can be evaluated twice, potentially producing labels inconsistent with the fitted rows.

Review effort: Balanced
Findings: 2

Open (2)

• Evaluate subset once to prevent mismatched fit labels · New
• Evaluate subset once to prevent mismatched fit labels · New

What changed in this PR

Backports robust survival-strata labeling to the 3.5.x maintenance line.

Changes:

• Derives group labels from fitted survfit() strata.
• Handles groups removed by subset or start.time.
• Adds regression coverage and release notes.

File	Description
R/​kaplan.R	Uses fitted strata for Kaplan labels.
R/​nelson.R	Applies equivalent Nelson labeling.
R/​utils.R	Adds strata and fit-row helpers.
tests/​testthat/​test_kaplan_nelson.R	Covers subset and start-time cases.
NEWS.md	Documents the corrected behavior.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[ehrlinger]

Checked, and it does not happen. A ... argument is a promise: survfit()'s model-frame call forces it, R caches the value, and list(...)$subset afterwards reads that cached value without evaluating the expression again. I tested it with a subset that counts its own evaluations:

n_eval <- 0
kaplan("time", "status", data = veteran, by = "trt",
       subset = { n_eval <- n_eval + 1; veteran$trt == n_eval })

n_eval is 1 afterwards for both kaplan() and nelson() (including nelson()'s second, weighted fit), on this branch and on main, and the result is labelled group 1, matching the fit. A random single-group subset also came back with the label of the group the fit kept. No change made.

[ehrlinger]

Checked, and it does not happen. A ... argument is a promise: survfit()'s model-frame call forces it, R caches the value, and list(...)$subset afterwards reads that cached value without evaluating the expression again. I tested it with a subset that counts its own evaluations:

n_eval <- 0
kaplan("time", "status", data = veteran, by = "trt",
       subset = { n_eval <- n_eval + 1; veteran$trt == n_eval })

n_eval is 1 afterwards for both kaplan() and nelson() (including nelson()'s second, weighted fit), on this branch and on main, and the result is labelled group 1, matching the fit. A random single-group subset also came back with the label of the group the fit kept. No change made.