docs: surface aggregate_survey() and fix stale references#288
Conversation
aggregate_survey() shipped in v3.0.1 but multiple roadmap and strategy docs still called it survey_aggregate() (the proposed name) and marked it as future work. Practitioner-facing docs (README, getting started, decision tree, choosing estimator, survey-roadmap) made no mention of the helper at all, leaving anyone with BRFSS/ACS/CPS microdata unable to discover the preprocessing step. Staleness fixes (wrong name + wrong status): - ROADMAP.md: B3d row marked shipped, deleted "Future: Survey Aggregation Helper" section - docs/business-strategy.md: corrected wrong-name references in lines 332 and 376; line 376 strikethrough preserves the historical name Discoverability additions: - docs/survey-roadmap.md: new "v3.0.1: Survey Aggregation Helper" entry in the canonical survey doc - README.md: new bullet in "For Data Scientists" plus a feature bullet - docs/index.rst: new :func: link in Quick Links; also adds the previously orphaned tutorials/16_survey_did and tutorials/16_wooldridge_etwfe to the Advanced Methods toctree - docs/choosing_estimator.rst: new .. note:: in Survey Design Support pointing at the preprocessing step - docs/practitioner_getting_started.rst: new microdata paragraph plus a runnable code example in "What If You Have Survey Data?" - docs/practitioner_decision_tree.rst: new microdata paragraph in the Survey Data section Documentation dependency tracking: - docs/doc-deps.yaml: prep.py entry now lists all 5 affected docs, matching the prep_dgp.py pattern TODO.md catalog updates from sphinx warning audit: - Bumped duplicate-object-description count from ~376 to ~1583 - New Medium entry for the 3 failed-import warnings (DiDResults.ci, MultiPeriodDiDResults.att, CallawaySantAnnaResults.aggregate) - New Low entry for the EDiDBootstrapResults cross-reference ambiguity - New Low entry for tracked autosummary stub staleness Validation: - sphinx-build exit 0; warnings dropped from 1723 to 1721 (the 2 orphan tutorial warnings are gone) - Zero new warnings introduced; zero warnings on edited content - grep survey_aggregate returns no remaining stale function references Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Overall Assessment Executive Summary
Methodology
Code Quality
Performance
Maintainability
Tech Debt
Security
Documentation/Tests
Assumption
Path to Approval
|
Address P1 review feedback on PR #288: aggregate_survey() returns a SurveyDesign with weight_type="aweight", but pweight-only estimators (CS, ImputationDiD, TwoStageDiD, StackedDiD, TripleDifference, StaggeredTripleDifference, SyntheticDiD, TROP, WooldridgeDiD) raise ValueError on aweight. The previous wording told practitioners they could fit "any estimator" with the returned stage2 design, which would crash at fit time. - README.md: explicit list of compatible estimators (Full in matrix) - docs/choosing_estimator.rst: rewrite the .. note:: with the same list and an explicit warning about pweight-only estimators - docs/practitioner_getting_started.rst: rewrite prose + switch the worked example from DifferenceInDifferences to SunAbraham, the only modern staggered estimator marked Full - docs/practitioner_decision_tree.rst: add the same caveat for consistency (not flagged by reviewer but same gap) - TODO.md: new Methodology/Correctness entry tracking the broader product gap (modern staggered methods need a manual SurveyDesign reconstruction; consider opt-in pweight mode for aggregate_survey) Verified: sphinx-build exit 0, 1721 warnings (same as before, no new warnings introduced); SunAbraham reference resolves; cross-references to :ref:\`survey-design-support\` resolve. Reviewer: #288 (comment) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
/ai-review |
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall Assessment ✅ Looks good The prior P1 from the last review is resolved. The edited user-facing docs now correctly say that Executive Summary
Methodology
Code Quality
Performance
Maintainability
Tech Debt
Security
Documentation/Tests
|
Summary
aggregate_survey()shipped in v3.0.1 but multiple roadmap and strategy docs still called itsurvey_aggregate()(the proposed name) and marked it as future work. Practitioner-facing docs (README, getting started, decision tree, choosing estimator, survey-roadmap) made no mention of the helper at all, leaving anyone with BRFSS/ACS/CPS microdata unable to discover the preprocessing step.Staleness fixes (wrong name + wrong status):
ROADMAP.md: B3d row marked shipped, deleted "Future: Survey Aggregation Helper" sectiondocs/business-strategy.md: corrected wrong-name references at lines 332 and 376; line 376 strikethrough preserves the historical nameDiscoverability additions:
docs/survey-roadmap.md: new "v3.0.1: Survey Aggregation Helper" entry in the canonical survey docREADME.md: new bullet in "For Data Scientists" plus a feature bulletdocs/index.rst: new:func:link in Quick Links; also adds the previously orphanedtutorials/16_survey_didandtutorials/16_wooldridge_etwfeto the Advanced Methods toctreedocs/choosing_estimator.rst: new.. note::in Survey Design Support pointing at the preprocessing stepdocs/practitioner_getting_started.rst: new microdata paragraph plus a runnable code example in "What If You Have Survey Data?"docs/practitioner_decision_tree.rst: new microdata paragraph in the Survey Data sectiondocs/doc-deps.yaml:prep.pyentry now lists all 5 affected docs, matching theprep_dgp.pypatternTODO.md catalog updates from sphinx warning audit:
~376to~1583DiDResults.ci,MultiPeriodDiDResults.att,CallawaySantAnnaResults.aggregate)EDiDBootstrapResultscross-reference ambiguityMethodology references (required if estimator / math changes)
Validation
sphinx-buildexit 0; total warnings dropped from 1723 → 1721 (the 2 orphan-tutorial warnings are eliminated)grep survey_aggregatereturns no remaining stale function references in user docs (the only matches are an intentional historical strikethrough inbusiness-strategy.mdand a test method nametest_survey_aggregate_and_summaryintests/test_wooldridge.pythat refers to theWooldridgeDiD.aggregate()method, not the helper)Security / privacy
Generated with Claude Code