Skip to content

Add practitioner decision tree and getting started guide (B1b-d)#287

Open
igerber wants to merge 1 commit intomainfrom
ds-practitioners-b1
Open

Add practitioner decision tree and getting started guide (B1b-d)#287
igerber wants to merge 1 commit intomainfrom
ds-practitioners-b1

Conversation

@igerber
Copy link
Copy Markdown
Owner

@igerber igerber commented Apr 10, 2026

Summary

  • Add business-framed estimator selection guide (docs/practitioner_decision_tree.rst) mapping 5 common scenarios (simultaneous campaign, staggered rollout, varying spend, few markets, survey data) to recommended estimators
  • Add end-to-end practitioner getting started guide (docs/practitioner_getting_started.rst) walking through a marketing campaign analysis with runnable code, validity checks, and stakeholder communication template
  • Add "For Data Scientists" README section with links to new docs
  • Add Sphinx toctree sections ("For Data Scientists", "Tutorials: Business Applications") and Quick Links
  • Update docs/doc-deps.yaml with dependency entries for 7 source files
  • Mark B1b, B1c, B1d as Done in ROADMAP.md

Methodology references (required if estimator / math changes)

  • N/A - no methodology changes. Documentation only.

Validation

  • Tests added/updated: No test changes (docs-only PR)
  • All code blocks in both RST files verified executable with python -W all (no warnings suppressed)
  • Sphinx build produces zero new warnings for new files
  • All :doc: cross-references resolve; all link targets verified to exist

Security / privacy

  • Confirm no secrets/PII in this PR: Yes

Generated with Claude Code

Phase B1 foundation docs making diff-diff discoverable for data science
practitioners. Business-framed estimator selection, end-to-end marketing
campaign walkthrough, and README entry point.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

Overall Assessment

⚠️ Needs changes

This is a docs-only PR, but the new practitioner decision tree and walkthrough make methodology-bearing estimator-selection and interpretation claims. I found two unmitigated P1 issues in that guidance, plus one broken survey example. I did not find matching REGISTRY.md deviation notes or TODO.md tracking that would mitigate them.

Executive Summary

  • Severity: P1. The new ContinuousDiD section presents dose-response / ATT(d) interpretation without the Strong Parallel Trends caveat required by the Methodology Registry.
  • Severity: P1. The new SyntheticDiD example describes treatment starting at period 7 but omits post_periods; the estimator will instead use its midpoint default and analyze a different post window.
  • Severity: P2. The new SurveyDesign example uses unsupported data= and weight= kwargs and will fail as written.

Methodology

  • Severity: P1. The ContinuousDiD guidance in docs/practitioner_decision_tree.rst:141 says the method gives a dose-response curve and that ATT(d) is “the lift at each spending level,” but the registry says plain PT identifies only ATT(d|d) / ATT^loc, while ATT(d) and ACRT(d) require Strong Parallel Trends docs/methodology/REGISTRY.md:465. The existing estimator guide already carries that caveat docs/choosing_estimator.rst:252. Impact: practitioners can over-interpret overall_att and the dose-response output as point-identified under weaker assumptions than the method requires. Concrete fix: add a short practitioner-facing note here that full dose-response interpretation requires SPT, plus the untreated-group / balanced-panel / time-invariant-dose conditions, or soften the text to the PT-identified quantity instead.
  • Severity: P1. The SyntheticDiD example in docs/practitioner_decision_tree.rst:183 narrates treatment starting at period 7, but SyntheticDiD.fit defaults post_periods to the last half of observed periods when that argument is omitted diff_diff/synthetic_did.py:190 diff_diff/synthetic_did.py:296. With 12 periods, that makes period 6 part of the post window. Impact: the example estimates a different estimand than the text describes and can silently report the wrong ATT. Concrete fix: pass the actual post periods explicitly, for example from the generated post column, or change the DGP so the midpoint default matches the narrative.

Code Quality

No findings in scope for a docs-only PR.

Performance

No findings in scope for a docs-only PR.

Maintainability

No findings. The docs/index.rst and docs/doc-deps.yaml additions look internally consistent.

Tech Debt

No separate deferrable tech-debt item. The blocking issues above are not mitigated by existing TODO.md entries.

Security

No findings; I did not see secrets, credentials, or PII introduced by the diff.

Documentation/Tests

  • Severity: P2. The survey snippet in docs/practitioner_decision_tree.rst:226 uses SurveyDesign(data=..., weight=...), but the actual dataclass begins at diff_diff/survey.py:27 and takes weights, strata, psu, fpc, etc.; it does not accept data or weight. Impact: the example raises TypeError as written, so the PR’s “code blocks verified executable” claim does not hold for this snippet. Concrete fix: change it to SurveyDesign(weights="sample_weight", strata="stratum", psu="cluster_id") (and fpc= if intended), then rerun snippet validation.
  • Severity: P3. docs/practitioner_getting_started.rst:120 says print(results) prints a summary table, but DiDResults.__repr__ is a one-line summary and the full table comes from summary() diff_diff/results.py:99. Impact: minor user confusion. Concrete fix: change the snippet to print(results.summary()) or adjust the prose.

Path to Approval

  1. Add an explicit ContinuousDiD note in docs/practitioner_decision_tree.rst stating that dose-response / ATT(d) interpretation requires Strong Parallel Trends, and mention the untreated-group, balanced-panel, and time-invariant-dose requirements.
  2. Fix the SyntheticDiD example so its post_periods matches the stated treatment start, either by passing the actual post-period list or by changing the generated DGP to align with the midpoint default.
  3. Correct the survey example to the real SurveyDesign API and rerun doc-snippet validation on the new .rst files.

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