Phase 2.1: incidence DX (rolling/cumulative + examples)

[mewincaka]

Phase 2.1: incidence DX improvements (rolling/cumulative + examples)

Key points

• Add optional cumulative=True for wide output.
• Add optional rolling=<int> for wide output with rolling_kind (sum/mean).
• Expand README examples for incidence.

Why this implementation

• These options are common in epi workflows (cumulative incidence, smoothed curves) and improve usability without changing the core incidence semantics.
• Implemented at the wide-output layer so behavior is consistent across strata columns.

Multi-role debate (differences, not consensus)

Role A — pragmatic developer

• 👍 Likes: one-liner usability; fewer pandas steps in notebooks.
• ⚠️ Concern: incidence() may grow into a kitchen-sink API if we keep adding transforms.

Role B — architecture

• Position A: Keep transforms inside incidence()
  • 👍 Pro: simple entry point; fewer functions to learn.
  • ⚠️ Con: API surface balloons (rolling params, cumulative params, normalization, etc.).
• Position B: Keep incidence() minimal; move transforms to a separate function
  • 👍 Pro: stable core primitive; transformations become composable building blocks.
  • ⚠️ Con: slightly more code for users (call 2 functions).

Role C — developer user (DX)

• 👍 Likes: incidence(..., rolling=..., cumulative=True) is extremely convenient.
• ⚠️ Concern: wants more control (min_periods, center, cumulative_by), and expects these to work for output="long" too.

Points of divergence to revisit (explicit)

1. Long-term API shape:
   • Option 1: keep transforms in incidence() but cap scope (only rolling/cumulative).
   • Option 2: introduce transform_incidence() and eventually deprecate transform flags on incidence().
2. Add min_periods, center, cumulative_by.
3. Apply transforms to output="long" as well.

Proposed next step

• If you prefer Option 2, I will follow up with a small refactor PR:
  • incidence() returns counts only (no rolling/cumulative)
  • transform_incidence(df, rolling=..., cumulative=..., ...) handles both wide+long
  • Keep current flags temporarily (deprecated) if we want a smoother transition.

[kengggg]

Interesting divergence.

1. Whether these transforms belong inside incidence() long-term.

I feel that making incidence() too big would be blasting API. Find the way and debate again what should be done.

2.Add min_periods, center, and cumulative_by options.

Good idea

3. Apply transforms to output="long" as well.

Good idea.

Also please debate everything before presenting implement options.

[mewincaka]

Thanks — acked. I’ll do a second round of debate and propose a cleaner shape so incidence() doesn’t become a “kitchen sink”.

I’ll come back with:

• a revised debate section that contrasts: (A) keep transforms in incidence() vs (B) move them to a separate transform_incidence() (or similar)
• concrete options + what we’d deprecate/keep
• follow-up PR changes (if we decide to refactor)

ETA: later today.

[mewincaka]

Ack — you’re right about tracking.

WHY

• If we keep adding transforms into incidence(), the function signature will balloon (rolling/min_periods/center/cumulative_by/normalize/etc.) and become harder to learn and maintain.
• But users do benefit from 1-liners for common workflows.

WHAT (options)

• Option A (keep in incidence(), capped): keep only rolling+cumulative flags (maybe add min_periods/center) and explicitly stop scope creep there.
• Option B (separate transform): keep incidence() as “counts only”; add transform_incidence() that can apply rolling/cumulative (and later min_periods/center/cumulative_by) to both wide+long outputs.

My recommendation: Option B (cleaner architecture, easier to extend without bloating incidence()).

ETA

• Within ~2–3 hours I can push a follow-up commit (or a new PR if you prefer) implementing Option B with backward-compat (temporarily keep current flags but mark as deprecated in docstring).

Tell me whether you prefer Option A or B, and whether you want the refactor in this PR#5 or as PR#6.

[kengggg]

@notskynet-bot Option B is cleaner. Please go ahead with that. Make sure the DX is distinguishable from both of the functions.

[mewincaka]

Ack — got it. I’m implementing Option B now.

WHY

• Keep incidence() as the stable “counts primitive” (plus output/fill_missing).
• Move rolling/cumulative (and future min_periods/center/cumulative_by) into transform_incidence() so we don’t bloat the incidence API.

WHAT (plan)

• Add transform_incidence() that works for both output="wide" and output="long".
• Update docs + tests to use transform_incidence(incidence(...), ...).
• Keep the DX clearly distinguishable: incidence() = counts; transform_incidence() = time-series transforms.

ETA

• Push updates to PR#5 within ~2 hours, with local ruff+pytest green before pushing.

[mewincaka]

Ack — taking action now.

WHY

• To keep incidence() from ballooning, transforms should live in a separate function.

WHAT

• Refactored PR#5 to implement Option B:
  • incidence() now returns counts only (plus output/fill_missing).
  • New transform_incidence() applies rolling/cumulative (and is the home for future min_periods/center/cumulative_by).
  • Updated tests + README examples to use transform_incidence(incidence(...), ...).

ETA

• CI is running now; once all checks are green I’ll ping you to review.

Commit: f5b4e12

[mewincaka]

CI is green ✅ — ready for review: #5

[kengggg]

@notskynet-bot looking good. approved.