docs: correct stale R-comparison-tests TODO row (R not in CI) by igerber · Pull Request #535 · igerber/diff-diff · GitHub
Skip to content

docs: correct stale R-comparison-tests TODO row (R not in CI)#535

Merged
igerber merged 1 commit into
mainfrom
perf/rscript-consolidation
Jun 8, 2026
Merged

docs: correct stale R-comparison-tests TODO row (R not in CI)#535
igerber merged 1 commit into
mainfrom
perf/rscript-consolidation

Conversation

@igerber

@igerber igerber commented Jun 7, 2026

Copy link
Copy Markdown
Owner

Summary

  • Correct a stale TODO row (Testing/Docs, Fix TWFE within-transformation bug and add methodology review #139): "R comparison tests spawn separate Rscript per test (slow CI)". The premise is false — no CI workflow installs R (no setup-r / r-lib/actions / fixest / r-base anywhere in .github/workflows/), so every R-parity test skips in CI behind a per-file availability gate (fixest_available, _check_r_contdid(), require_r/r_available). Consolidating Rscript spawns therefore yields zero CI speedup.
  • The originally-cited file (tests/test_methodology_twfe.py) already session-caches its R fits via @pytest.fixture(scope="session") (r_twfe_results / r_twfe_results_with_covariate).
  • Rewrite the row to document the corrected scope: the only residual is a local-dev-only micro-optimization (continuous_did.py / callaway.py re-spawn library(...) per call with no session cache). Retained as a low-value local-dev note rather than silently dropped, so it isn't re-raised as a CI-speed item.
  • Drop the already-shipped "doc-deps integrity CI" (PR chore: add CI integrity check for docs/doc-deps.yaml #519) from the dangling Tier-D parenthetical — its dedicated row was removed when chore: add CI integrity check for docs/doc-deps.yaml #519 merged but the mention was left behind.

Methodology references

  • N/A — documentation-only change (TODO.md). No estimator, identification, weighting, or variance/SE behavior is touched.

Validation

  • No test changes. Claims were verified against the repo before writing: grep confirms no R in .github/workflows/; the twfe session-scoped fixtures, the continuous_did / callaway call sites, and the removed chore: add CI integrity check for docs/doc-deps.yaml #519 doc-deps row/test (tests/test_doc_deps_integrity.py exists) were each confirmed.
  • Local AI review (codex, gpt-5.5, full standard, --force-fresh): ✅ clean — no P0/P1/P2; one P3-informational note confirming the reframing is correct.

Security / privacy

  • Confirm no secrets/PII in this PR: Yes

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Jun 7, 2026

Copy link
Copy Markdown

…c-deps mention

The "R comparison tests spawn separate Rscript per test (slow CI)" row's premise
is false: no CI workflow installs R (no setup-r / r-lib/actions / fixest / r-base
in .github/workflows/), so every R-parity test skips in CI behind a per-file
availability gate. The originally-cited test_methodology_twfe.py already
session-caches its R fits via scope="session" fixtures. Rewrite the row to
document the correction (the only residual is a local-dev-only micro-optimization
in continuous_did / callaway, which re-spawn library() per call with no session
cache) and drop the already-shipped "doc-deps integrity CI" (#519) from the
dangling Tier D parenthetical.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@igerber igerber force-pushed the perf/rscript-consolidation branch from bbb7cf5 to 6290ca1 Compare June 7, 2026 18:06
@github-actions

github-actions Bot commented Jun 7, 2026

Copy link
Copy Markdown

@igerber igerber added the ready-for-ci Triggers CI test workflows label Jun 8, 2026
@igerber igerber merged commit ea5d405 into main Jun 8, 2026
5 of 6 checks passed
@igerber igerber deleted the perf/rscript-consolidation branch June 8, 2026 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-ci Triggers CI test workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant