docs: custom autosummary template + residual triage (PR 2.5)#410
docs: custom autosummary template + residual triage (PR 2.5)#410
Conversation
Closes the gap PR #407 left: 614 of 635 PR #407-residual warnings were member-level duplicate-object descriptions because :no-index: on page-level autoclass directives doesn't propagate to autodoc-generated members. PR 2.5 adds a custom autosummary class.rst template with :no-members: on the stub autoclass, and a function.rst template with :no-index: on the stub autofunction. All 50 class stubs and 48 function stubs are auto-regenerated on the next clean build (autosummary_generate_overwrite=True default). Architecture: - Class cross-refs resolve to stub URL (per PR #407 page-level :no-index:) - Member cross-refs resolve to page-level URL (only place still registered) - Function cross-refs resolve to page-level URL (stub now :no-index:) - Page-level pages keep current full inline member rendering (UX preserved) - Stubs become "header + autosummary tables linking to page-level" Also addresses 18 non-duplicate residuals so make html now warns 0: - 4 footnote-not-referenced + 1 def-list blank-line in source docstrings - 4 :doc: refs to .md files (myst-parser absent) converted to literal text - 4 broken markdown links to REGISTRY.html in T18/T19 notebooks - 3 orphan benchmarks.rst citations removed - T19 + T20 added to Tutorials: Business Applications toctree - 2 stale autosummary entries fixed (DiDResults.ci, CallawaySantAnnaResults.aggregate) - 3 page-vs-page function dups (chaisemartin_dhaultfoeuille, stacked_did, trop) resolved by adding :no-index: to page-level autofunction directives and switching 3 path-qualified autoclass directives to top-level paths Result: Sphinx make html went 2,170 (PR #407 start) -> 0 warnings (PR 2.5 end). This unblocks PR 3 (queued: -W enforcement in docs-tests.yml). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
✅ Looks goodExecutive Summary
MethodologyNo P0/P1 findings. Affected methods checked:
Code QualityNo P0/P1/P2 findings. Pattern-wide inference grep found no new inline inference computation in the changed Python hunks. Existing project-wide hits are outside this PR’s changed code. PerformanceNo findings. The PR is documentation/template focused and does not alter runtime estimator paths. MaintainabilityNo P0/P1 findings. P3 — Informational: tracked docs tech debt addressed Impact: The autosummary refresh and custom templates directly address existing Sphinx duplicate/stale-stub debt already tracked in Concrete fix: No action required. Tech DebtNo untracked blocker-level tech debt found. The remaining limitation around documentation inventory below is not statistical correctness and does not block approval. SecurityNo findings. I grepped the non-notebook diff for common secret patterns and did not find credentials or private keys. Documentation/TestsP2 — Shadowed convenience functions lose Python-domain targets Impact: The three page-level convenience functions that shadow module names are now documented with Concrete fix: Keep exactly one indexed target for each shadowed function. A clean shape is to replace the page P3 — Validation not locally reproduced Impact: I did not run Concrete fix: Rely on CI’s Sphinx build for final confirmation, or run |
Reviewer flagged that page-level :no-index: on .. autofunction:: (added
in initial PR 2.5 commit to resolve 3 page-vs-page duplicates) made
diff_diff.{trop,stacked_did,chaisemartin_dhaultfoeuille} convenience
functions absent from the py:function inventory entirely - so :py:func:
cross-refs and intersphinx pointers to those public functions wouldn't
resolve.
Empirical investigation surfaced the actual conflict source: each of the 3
files had a top-level ``.. module:: diff_diff.<name>`` directive that
registered the module FQN, colliding with the function FQN of the same
name. Removing the .. module:: directive lets .. autofunction:: register
the function as the canonical (and only) :py:function entry. The
.. autoclass:: directives on those pages were already converted to
top-level paths (diff_diff.X form) in the initial PR 2.5 commit and
don't depend on the implicit module context.
Verified: make html shows 0 ERRORs / 0 WARNINGs / 0 duplicates; all 3
convenience functions present in objects.inv as py:function entries.
Doc-snippet test parity preserved (111 passed, 4 skipped).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
/ai-review |
|
🔁 AI review rerun (requested by @igerber) Head SHA: ✅ Looks goodExecutive Summary
MethodologyNo P0/P1 findings. Severity: P3-informational Concrete fix: No action required. Severity: P3-informational Concrete fix: No action required. Code QualityNo P0/P1/P2 findings. Pattern-wide grep for inline inference anti-patterns ( PerformanceNo findings. The PR is documentation and Sphinx-template focused; no runtime estimator path changes were introduced. MaintainabilityNo P0/P1 findings. Severity: P3-informational Concrete fix: No action required. Tech DebtNo unmitigated blocker-level tech debt found. The docs-warning cleanup is already covered by SecurityNo findings. Secret-pattern grep over changed non-notebook source/docs found no obvious credentials, private keys, or tokens. Documentation/TestsSeverity: P2 Concrete fix: Add indexed page-level directives for the three functions, likely Severity: P3-informational Concrete fix: Rely on CI’s Sphinx build, or run |
R2 reviewer flagged 3 plotting helpers (plot_bacon, plot_power_curve, plot_pretrends_power) as having :no-index: stubs without page-level indexed replacements. Added page-level .. autofunction:: directives in visualization.rst, power.rst, and pretrends.rst. Proactive sweep (per feedback_ci_reviewer_pattern_checks) surfaced 7 sibling dataset loaders (load_card_krueger, load_castle_doctrine, load_divorce_laws, load_mpdta, load_dataset, list_datasets, clear_cache) with the same class of issue: docs/api/datasets.rst used bare names under .. module:: diff_diff.datasets, registering them at diff_diff.datasets.X instead of the canonical diff_diff.X export FQN. Removed the .. module:: directive and switched to fully-qualified diff_diff.X names so they appear at the same FQN as their autosummary listings. Verified: full coverage sweep finds 51/51 functions and 59/59 classes in py:function / py:class inventory under their canonical diff_diff.X FQN. make html shows 0 ERRORs / 0 WARNINGs / 0 duplicates. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
/ai-review |
|
🔁 AI review rerun (requested by @igerber) Head SHA: ✅ Looks goodExecutive Summary
MethodologySeverity: P3-informational Severity: P3-informational Code QualityNo P0/P1/P2 findings. Pattern-wide grep for inline inference anti-patterns found no new affected Python hunks. The only Python changes are docstrings. PerformanceNo findings. The PR is documentation/Sphinx infrastructure only; no runtime estimator path changes were introduced. MaintainabilityNo P0/P1/P2 findings. Static mirror checks passed: all class stubs have Workflow transitive-dependency audit: no workflow or pytest-selection changes were in the supplied diff. Existing docs workflow paths already include docs, package code, Tech DebtSeverity: P3-informational SecurityNo findings. Secret-pattern scan over changed Python/RST/Markdown surfaces, excluding the notebook carve-out paths, found no obvious credentials or private keys. Documentation/TestsNo P0/P1/P2 findings. Severity: P3-informational Severity: P3-informational |
1 participant
Summary
Closes the Sphinx WARNING gap PR #407 left. PR #407 dropped warnings from 2,170 to 635 (71%) by adding `:no-index:` to all 78 page-level `.. autoclass::` directives. The empirical spike on `bacon.rst` revealed that `:no-index:` on autoclass suppresses CLASS-level cross-reference registration only — it doesn't propagate to autodoc-generated members when `:members:` is implicit (via `autodoc_default_options["members"] = True` in `docs/conf.py:39-44`). 614 of those 635 residuals were member-level duplicates.
PR 2.5 closes both that gap and the 21 non-duplicate residuals so PR 3 (queued: `-W` enforcement in `docs-tests.yml`) can land cleanly.
Result: `make html` is now 0 ERRORs / 0 WARNINGs (down from 2,170 at PR #407 start; 635 at PR #407 merge).
Architecture
User-decided this session: custom autosummary class template that adds `:no-members:` to the autoclass in stubs (Option A). Effect:
Sphinx's `autosummary_generate_overwrite=True` default means a single clean build regenerates all 50 class stubs + 48 function stubs from the new templates — no hand-edit per stub.
Spike-before-bulk discipline
Per `feedback_spike_before_bulk_mechanical_edit` (lesson saved from PR #407): the plan included an explicit empirical spike of just the class.rst template. The spike caught that 41 of 44 remaining residuals were FUNCTION duplicates (not class), surfacing the need for a function.rst template too. The remaining 3 page-vs-page were pinned to specific module-name conflicts (`diff_diff.chaisemartin_dhaultfoeuille`, `diff_diff.stacked_did`, `diff_diff.trop` — function names that shadow module names) and resolved with targeted page-level `:no-index:` on the autofunction. Without the spike, the bulk pass would have committed to a 1-template architecture that left ~44 function dups unhandled.
Constructor visibility note
The new class template removes the explicit `.. automethod:: init` line from stubs (would re-register `init` and defeat the `:no-members:` suppression). The constructor signature still appears on page-level via `autodoc_class_signature = "separated"` (`docs/conf.py:46`). Stubs lose the per-class `init` rendering; page-level keeps it.
Residual triage (Cat A–E)
Five categories of non-duplicate warnings, all fixed:
Plan-reviewer found that the original Cat E plan (suggested `suppress_warnings = ["autosummary"]` based on "Rust backend not built") was wrong: Sphinx emits autosummary import-failure warnings without `type=`, so the suppress filter is a no-op. The actual fixes were RST typos.
Page-vs-page function fix (Cat F surprise)
Spike rebuild surfaced 3 final duplicates from `chaisemartin_dhaultfoeuille`, `stacked_did`, `trop` — function names that shadow their module names. The conflict was Python's same-namespace ambiguity: `from .X import X` shadows the module, but Sphinx still registers both. Fix: `:no-index:` on the page-level autofunction (3 sites). Also switched 3 path-qualified autoclass directives to top-level paths (`diff_diff.X.Y` → `diff_diff.Y`) for consistency with project convention (PR #406 export pattern).
Methodology references
Validation
Security / privacy