Skip to content

Fix 10 API discrepancies in quickstart.rst#103

Merged
igerber merged 2 commits intomainfrom
docs/fix-quickstart-api-discrepancies
Jan 23, 2026
Merged

Fix 10 API discrepancies in quickstart.rst#103
igerber merged 2 commits intomainfrom
docs/fix-quickstart-api-discrepancies

Conversation

@igerber
Copy link
Copy Markdown
Owner

@igerber igerber commented Jan 23, 2026

Summary

  • Fix generate_did_data() parameter: treatment_starttreatment_period
  • Fix DifferenceInDifferences.fit() parameters: treatedtreatment, posttime
  • Fix DifferenceInDifferences constructor: cluster_colcluster
  • Fix TwoWayFixedEffects.fit() parameter: treatedtreatment
  • Fix MultiPeriodDiD: move reference_period from constructor to fit()
  • Fix MultiPeriodDiD.fit(): replace treatment_start with post_periods
  • Fix CallawaySantAnna results attribute: attoverall_att
  • Fix check_parallel_trends(): correct import path and parameters
  • Fix HonestDiD: update to use method/M instead of delta=DeltaRM, and ci_lb/ci_ub instead of robust_ci
  • Fix plot_event_study import path: diff_diffdiff_diff.visualization

Methodology references (required if estimator / math changes)

  • N/A - documentation-only changes, no methodology code modified

Validation

  • Tests added/updated: No test changes needed (documentation only)
  • All 10 API fixes verified against actual implementation using Python introspection
  • Imports and parameter names confirmed correct via automated checks

Security / privacy

  • Confirm no secrets/PII in this PR: Yes

Generated with Claude Code

Update code examples to use correct parameter names:
- generate_did_data: treatment_start → treatment_period
- DifferenceInDifferences.fit: treated → treatment, post → time
- DifferenceInDifferences: cluster_col → cluster
- TwoWayFixedEffects.fit: treated → treatment
- MultiPeriodDiD: move reference_period from constructor to fit()
- MultiPeriodDiD.fit: treatment_start → post_periods
- CallawaySantAnna results: att → overall_att
- check_parallel_trends: fix import path and parameters
- HonestDiD: delta=DeltaRM → method/M, robust_ci → ci_lb/ci_ub

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

Overall Assessment: ⚠️ Needs changes

Executive Summary

  • Docs-only update; no estimator/math changes; Methodology Registry unaffected.
  • Most API name fixes now align with in-code signatures (DiD, TWFE, MultiPeriodDiD, HonestDiD).
  • Event study example still errors with the quickstart data because reference_period=-1 is not present in period.

Methodology

  • No findings.

Code Quality

  • No findings.

Performance

  • No findings.

Maintainability

  • No findings.

Tech Debt

  • No findings.

Security

  • No findings.

Documentation/Tests

  • P2 | Impact: Event-study snippet raises ValueError when using the earlier quickstart data (periods 0–9) because reference_period=-1 is not in the time column. | Concrete fix: set reference_period to an existing pre-period (e.g., 4) or add create_event_time(...) and use time='event_time' with reference_period=-1. (docs/quickstart.rst:127)

Change reference_period=-1 to reference_period=4 to match the data
generated by generate_did_data(n_periods=10), which has periods 0-9.
Period 4 is the last pre-treatment period (treatment starts at 5).

Addresses reviewer feedback from PR #103.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@igerber
Copy link
Copy Markdown
Owner Author

igerber commented Jan 23, 2026

/ai-review

@github-actions
Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: ff0c55784fa6ac5cd0b539d7ca0d2704b7f12266


Overall assessment: ✅ Looks good

Executive Summary

  • Quickstart examples now align with current API names and return values; no estimator logic changed.
  • Methodology registry alignment is unaffected because the diff is documentation-only.
  • Doc examples were not executed (residual risk: runtime drift if APIs change later).

Methodology

  • No findings.

Code Quality

  • No findings.

Performance

  • No findings.

Maintainability

  • No findings.

Tech Debt

  • No findings.

Security

  • No findings.

Documentation/Tests

  • No findings.

@igerber igerber merged commit 6791431 into main Jan 23, 2026
@igerber igerber deleted the docs/fix-quickstart-api-discrepancies branch January 23, 2026 22:28
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