Skip to content

feat: TWFE weight diagnostics (twfeweights R port, re-scoped from #753) - #812

Open
wenddymacro wants to merge 5 commits into
igerber:mainfrom
wenddymacro:feat/twfeweights-diagnostics
Open

feat: TWFE weight diagnostics (twfeweights R port, re-scoped from #753)#812
wenddymacro wants to merge 5 commits into
igerber:mainfrom
wenddymacro:feat/twfeweights-diagnostics

Conversation

@wenddymacro

Copy link
Copy Markdown
Contributor

Re-scoped follow-up to #753, branched off current main. Only twfeweights
(MIT, © 2023 Brantly Callaway); ptetools and badcontrols are dropped
entirely per your license read.

Against your six requirements

1. Post-lasso dropped. did_post_lasso / did_post_lasso_ra /
PostLassoResult are not ported — R/did_post_lasso.R:69 has the leftover
browser() and references undefined variables, so there is no runnable
reference. No sklearn dependency anywhere. No quality_reports/.

2. API consolidated to 5 public symbols (from 21 upstream exports):

Symbol Folds
attgt_weights(results, aggregation="twfe"|"overall"|"simple") twfe_weights, attO_weights, att_simple_weights
decompose_twfe_weights(data, ..., method="fwl") implicit_twfe_weights
ATTGTWeightsResult, TWFEDecompositionResult the 5 upstream containers
plot_twfe_weights() the 4 ggtwfeweights S3 methods

Covariate balance is a result-object method — result.covariate_balance(level="summary"|"cell")
— rather than R's mutate-in-place second pass, so the result never retains the
raw panel. Everything else is private; the two-period kernels and the four
balance statistics are pinned directly by the parity suite since they have no
public surface.

3. House conventions. attgt_weights takes a fitted
CallawaySantAnnaResults as the primary input, reading cohort masses off its
aggregation bookkeeping so no raw panel is needed; the fallback consumes
result.to_dataframe("group_time") verbatim. Params are
outcome/unit/time/first_treat. Both results subclass Diagnostic with
summary()/to_dict()/to_dataframe() and carry no inference quintet — the
headline scalars are named implied_att and estimate deliberately. Output
columns are ours (group, time, post, weight, att), never
time.period/attgt. Plotting is plot_twfe_weights() in
diff_diff/visualization/.

4. Output parity, and the fixest waiver is gone. I found the root cause:
fixest::demean() segfaults on a zero-column matrix, and xformula = ~1
is the only branch that builds one (model.matrix(~-1, data)nT × 0).
Reproduced in isolation on R 4.6.1 / fixest 0.14.2. It is not a property of any
fixture.

The no-covariate golden is therefore generated with a time-invariant
covariate — double-demeaning annihilates it exactly, so the call is
numerically the ~1 branch — and the parity test asserts both
covariates=None and covariates=[<that column>] against that one golden, so
the equivalence is proven rather than assumed. On mpdta,
twfe_weights(att_gt(...)) and implicit_twfe_weights(xformula = ~lpop)$est
both give -0.03654894.

Goldens: benchmarks/data/twfeweights_golden.json + three sibling panel CSVs,
regenerated by benchmarks/R/generate_twfeweights_golden.R. R is never needed
to run the tests; they pytest.skip if a fixture file is absent. Tolerances
are module constants with per-gate rationale, tabulated in REGISTRY.

Three fixtures: mpdta (real, non-1..T labels), sim_staggered (equal
cohorts, a real pre-trend so pretrend_bias != 0), and unbalanced_cohorts
(120/70/60 — breaks the p_g == 1/3 degeneracy that would let a cohort-share
bug pass silently on the equal-cohort fixture).

Results: ATT(g,t) weights match at machine precision (max 4.7e-16) on all
3 fixtures × 3 aggregations; FWL estimate and cell weights likewise; all 11
balance statistics at machine precision.

5. Docs. REGISTRY section (equations, cross-surface identity, tolerance
table, 11 Note/Deviation entries), docs/api/twfe_weights.rst, 4
api/index.rst registrations, doc-deps.yaml, references.rst, one README
line, llms.txt + llms-full.txt, changelog fragment. MIT attribution: the
upstream copyright notice is reproduced verbatim in the module docstring.

6. Branched off current main.

Two places I deliberately differ from R (both in REGISTRY)

Both were found by disagreeing with the goldens and then working out which
side was right.

Annihilated covariates are dropped before the projection. A time-invariant
regressor leaves a column of pure rounding noise after double-demeaning
(~1e-16 against a raw scale of ~1); regressing on it amplifies that by ~1e16
and silently corrupts the per-cell weights. The test is scale-relative
(demeaned norm vs the column's own raw norm) because a rank test on the
demeaned matrix alone cannot see it — there, 1e-16 is simply the largest
pivot. With this, covariates=None and covariates=[<time-invariant col>]
agree to 1e-15.

Cells with a 0/0 normalizer report the limit, not the noise. For the
never-treated group the double-demeaned treatment is constant within a period
(-E_t[D] + mean_t E_t[D]), and on sim_staggered that constant is
analytically zero at t=3 (-1/3 + 1/3). We take the limit (a constant
over its own mean is one); R divides the two rounding errors and lands ~3e-4
away. Checked against a hand-computed contrast that uses none of this module:
ours is exact to 4.4e-16. The aggregate is unaffected either way — the
weights on those cells cancel exactly (w(3,3) + w(4,3) = 0), which is why
estimate still matches R to 1e-15. The parity suite gates estimate tightly
everywhere and relaxes only the per-cell assertions, on cells detected as
degenerate rather than hard-coded to a fixture.

Naming — your call

I need names distinct from the existing twowayfeweights / TWFEWeightsResult
(dCDH) surface, which weights (unit, time) cells where these weight
ATT(g,t) parameters. Implemented as set A; B and C are mechanical renames
if you prefer one:

  • A (implemented): attgt_weights / ATTGTWeightsResult /
    decompose_twfe_weights / TWFEDecompositionResult. Puts the object being
    weighted in the head-noun position; shares no token with twowayfeweights
    and diverges at character 1 for autocomplete.
  • B: estimand_weights / EstimandWeightsResult /
    twfe_weight_decomposition / TWFEWeightDecompositionResult. Generalizes
    better if an aggregation="event_study" is added later; vaguer about what
    the rows are.
  • C: callaway_twfe_weights / … — author-prefixed. I did not use it since
    the library has BaconDecomposition, not goodman_bacon_decompose, and it
    reads confusingly next to CallawaySantAnna.

plot_twfe_weights is your name, kept verbatim.

Scope note

method= accepts "fwl" only. Upstream's implicit_aipw_weights is left for
a follow-up rather than growing this PR — the old port's AIPW was numerically
wrong against R (score form vs R's gamma0/gamma0_tilde AIPW-weights form),
so it needs a from-scratch implementation and its own parity work. The goldens
already carry the AIPW references, so that PR will not need R re-run. Recorded
in REGISTRY as a documented deviation, not a silent gap.

Verification

ruff / black clean, mypy diff_diff at zero errors, 14378 tests collect
clean. Green locally: the new suites (test_twfe_weights.py,
test_twfe_weights_parity.py), plus docs IA, doc-deps integrity, diagnostic
roster (M-091), guides, changelog fragments, serialization and all
visualization suites — 903 passed / 43 skipped. I did not complete a full
local pytest run (pure-Python mode, no Rust backend built locally); happy to
add ready-for-ci whenever you want CI on it.

🤖 Generated with Claude Code

yiyi and others added 5 commits August 31, 2026 09:20
Generator + committed goldens for the upcoming `attgt_weights` /
`decompose_twfe_weights` surface, ported from Brantly Callaway's
`twfeweights` R package (MIT).

Three fixtures: `mpdta` (real; non-1..T time labels), `sim_staggered`
(equal cohorts, real pre-trend so pretrend_bias != 0), and
`unbalanced_cohorts` (120/70/60 — breaks the p_g == 1/3 degeneracy that
would let a cohort-share bug pass silently on the equal-cohort fixture).

Pins `twfe_weights`/`attO_weights`/`att_simple_weights`,
`implicit_twfe_weights` (no-cov, covariate, gmin1), `implicit_aipw_weights`,
`twfe_cov_bal`/`aipw_cov_bal` + the summary roll-up, and the two two-period
kernels that will have no public Python surface.

The no-covariate decomposition is generated with a TIME-INVARIANT covariate
rather than `xformula = ~1`: upstream builds an nT x 0 model matrix on that
branch and `fixest::demean` segfaults on a zero-column matrix (reproduced in
isolation, fixest 0.14.2 / R 4.6.1). Double-demeaning annihilates a
time-invariant regressor exactly, so the call is numerically the `~1` branch
— and the Python test will assert both `covariates=None` and
`covariates=[<col>]` against this one golden, proving the equivalence rather
than assuming it.

R is only needed to regenerate the JSON, never to run the tests.

Co-Authored-By: Claude <noreply@anthropic.com>
…iners

Result containers for the incoming TWFE implicit-weight diagnostics, landed
ahead of the compute module so they pin the output schema and the Diagnostic
contract before any math depends on them.

Both subclass Diagnostic: they assess what a regression implicitly weights
rather than estimating an effect, so neither carries the estimator quintet.
The headline scalars are deliberately named `implied_att` and `estimate`
rather than `att` so they do not read as inference-bearing.

Output columns are diff-diff's (`group`, `time`, `post`, `weight`, `att`),
not R's (`time.period`, `attgt`).

Covariate balance is a result-object method rather than a mutate-in-place
second pass as in R: `covariate_balance(level="summary"|"cell")` reads a
table computed at construction time, so the result never retains the raw
panel. It raises with the fix inlined when balance was not requested.

Names are clearly separated from the existing dCDH surface
(`twowayfeweights` / `TWFEWeightsResult`), which weights (unit, time) cells;
these weight ATT(g,t) parameters.

Roster (M-091) and the shared construction fixture updated; the roster test
auto-enrolls both classes.

Co-Authored-By: Claude <noreply@anthropic.com>
One entry point folding R's three separate weight functions behind
`aggregation=`: "twfe" (twfe_weights), "overall" (attO_weights, ATT^O), and
"simple" (att_simple_weights, ATT^simple). Reports what each estimand
implicitly puts on every group-time effect, plus the negative-weight share
that makes the staggered-TWFE pathology legible.

Takes a fitted CallawaySantAnnaResults as the primary input, reading cohort
masses off the aggregation bookkeeping so no raw panel is needed; a
(gt_frame, data=, unit=, time=, first_treat=) fallback consumes
`result.to_dataframe("group_time")` verbatim.

Design restrictions are hard errors, not warnings, each naming its fix:
aggregation="twfe" needs base_period="universal" and
control_group="never_treated" (matching R's own stop()s), and no aggregation
accepts a repeated-cross-section or unbalanced-fallback fit, whose cohort
shares are not comparable across periods.

Deviation from R: cohorts and periods are mapped to positional time before
the (maxT - g + 1)/T arithmetic. R evaluates that on raw labels, which is
only correct on consecutive integers; positional time is bit-identical there
(mpdta 2003..2007 -> 1..5 both give 4/5 at g=2004) and correct on gapped
grids. Pinned by a test that remaps periods to 10,20,30,40,50.

R's keep_untreated= is not exposed: it synthesizes G=0 rows that are
excluded from every normalization and contribute exactly zero.

Parity: machine precision (max |dw| = 4.7e-16) against R twfeweights 0.9.0
on 3 fixtures x 3 aggregations. Primary assertions feed R's own ATT(g,t)
back in, isolating this module from CallawaySantAnna-vs-`did` parity; a
separate, deliberately looser class covers the composed end-to-end path.

Also fixes a generator bug: R stores `post` as a FACTOR, so as.integer()
emitted level codes 1/2 rather than 0/1.

Known-red until the docs commit: test_doc_deps_integrity.py wants a
docs/doc-deps.yaml entry, which lands with the API page.

Co-Authored-By: Claude <noreply@anthropic.com>
…ance

Re-derives a TWFE estimate from its ATT(g,t) building blocks, reporting the
implicit weight on each cell, the contribution of PRE-treatment cells
(`pretrend_bias` - parallel-trends violations rather than treatment), and
implicit-weight covariate balance via `result.covariate_balance()`.

Takes the raw panel rather than a fitted CS result because it re-estimates:
it double-demeans treatment and covariates and forms its own group-time
contrasts, so there is no ATT(g,t) table it could consume, and a CS result
carries no panel by design. The two surfaces are tied by an identity that
the suite pins:

    attgt_weights(cs, aggregation="twfe").implied_att
        == decompose_twfe_weights(panel, ...).estimate

Two numerical points, both found by disagreeing with the goldens and then
proving which side was right:

1. Covariates that double-demeaning ANNIHILATES are now dropped before the
   projection, judged against each column's own PRE-demeaning norm. A
   time-invariant regressor leaves a column of pure rounding noise (~1e-16
   against a raw scale of ~1); regressing on it amplifies that by ~1e16 and
   silently corrupted the per-cell weights. A rank test on the demeaned
   matrix alone cannot see this - there, 1e-16 is simply the largest pivot.
   With the fix, covariates=None and covariates=[<time-invariant col>] agree
   to 1e-15 on every fixture, which is exactly the equivalence the
   no-covariate golden relies on.

2. Cells whose comparison-group implicit weights are constant AND average to
   zero make `resid / mean(resid)` a 0/0. On sim_staggered (equal cohorts at
   g in {0,3,4}, T=5) this happens exactly at t=3, where
   -E_3[D] + mean_t E_t[D] = -1/3 + 1/3. We take the limit (a constant over
   its own mean is one); R divides the rounding errors and lands ~3e-4 away.
   Verified against a hand-computed contrast that needs none of this module:
   ours is exact to 4.4e-16. The weights on such cells cancel exactly in the
   aggregate, so `estimate` is unaffected - the suite gates `estimate`
   tightly on every fixture and relaxes only the per-cell and
   decomposition/remainder-split assertions, on cells DETECTED as degenerate
   rather than on a hard-coded fixture.

Parity vs R twfeweights 0.9.0: estimate and per-cell weights at machine
precision on all 3 fixtures x 4 configurations; all 11 balance statistics at
machine precision, including `frac_treated_extreme`, which required
reproducing BMisc's weighted-ECDF plus `stats:::quantile.ecdf`'s
pseudo-sample reconstruction rather than a plain quantile.

method="aipw" is not implemented yet and raises listing the accepted values.

Co-Authored-By: Claude <noreply@anthropic.com>
Plotting (replacing upstream's ggtwfeweights S3 methods) and every
documentation surface the new API owes.

plot_twfe_weights(result, kind="auto"|"weights"|"balance") lives beside
plot_bacon in visualization/_diagnostic.py and dispatches on either result
type. The weights view puts weight on x and ATT(g,t) on y with zero lines, so
negative-weight cells sit visibly left of the axis; the balance view plots
unweighted against implicitly-weighted covariate differences with a
no-improvement diagonal. "auto" picks balance when a balance table is
present.

Docs: a REGISTRY.md section carrying the weight equations, the cross-surface
identity, the tolerance table with per-gate rationale, and eleven explicit
Note/Deviation-from-R entries - including the fixest zero-column segfault and
its root cause, the annihilated-covariate drop, and the 0/0-cell limit, so
the two places we deliberately differ from R are recorded rather than
discovered later by a reviewer. Two paragraphs separate this surface from
`twowayfeweights` (dCDH, weights (unit, time) cells) and from
BaconDecomposition (decomposes into 2x2 comparisons), since all three are
"TWFE weight" diagnostics and the distinction is the thing a reader most
needs.

Also: docs/api/twfe_weights.rst with runnable examples, four api/index.rst
registrations (2 result classes, the plot, 2 functions, toctree),
doc-deps.yaml group + sources entries, a README one-liner in Diagnostics &
Sensitivity, llms.txt catalog entry, llms-full.txt API + result blocks, a
references.rst sub-entry naming the upstream package and its MIT copyright,
and a changelog.d fragment.

This closes the doc-deps gate the attgt_weights commit left red.

Verified: 14378 tests collect clean; docs IA, doc-deps integrity, diagnostic
roster, guides, changelog-fragment, serialization and all visualization
suites green (903 passed, 43 skipped).

Co-Authored-By: Claude <noreply@anthropic.com>
@igerber

igerber commented Sep 5, 2026

Copy link
Copy Markdown
Owner

Thanks for the re-scope - this is a strong port. I confirmed the upstream MIT license, ran the new suites locally (572 passed, no skips, so the goldens were exercised), and lint/black/mypy are clean. The weight formulas for all three aggregations, the FWL decomposition under both base periods, and all eleven balance statistics match R at machine precision on all three fixtures, and both of your documented deviations from R are mathematically right. The public surface, Diagnostic subclassing, column names, naming versus the dCDH surface, attribution, and the docs registrations all conform to what I asked for on #753.

Most of what follows is about the paths that go beyond the R reference - the ATT(g,t)-frame input, user-supplied weights, and non-standard cohort labels - where the port needs its own input validation because R never faced those inputs. Items 1 through 5 I reproduced by running the code. Please make the changes on this PR; once your revision is pushed I will open a maintainer-side mirror so the label-gated CI and the CI reviewer can run (they do not trigger on fork PRs), and I will bring anything CI surfaces back here.

Decisions on the two open questions

  • Naming: option A (as implemented). Keep it.
  • AIPW deferral: accepted. method="fwl" only is fine for this PR, with the cleanup in item 10 so nothing claims AIPW is present.

Items to address

  1. Cohort labels: NaN and -inf are silently treated as never-treated. _is_never uses ~np.isfinite, so a missing or -inf first_treat becomes cohort 0 and the within-unit consistency check ignores it. Setting one treated unit's label to NaN moved decompose_twfe_weights(...).estimate from 2.2796 to 2.2481 with no warning. Never-treated should be exactly 0 or +inf; reject everything else non-finite up front and include missing values in the unit-invariance check. Add a test.

  2. Balance summary turns NaN into 0. _frac_treated_extreme correctly returns NaN for a covariate with fewer than three distinct values (R's rule), but covariate_balance(level="summary") rolls cells up with a pandas .sum(), which skips NaN, so a binary or constant covariate reports unweighted_frac_extreme = 0.0 and weighted_frac_extreme = 0.0. R propagates NA. REGISTRY currently says the port "reproduces this exactly, including the NA return"; the summary does not. Propagate NaN through the roll-up and test both levels with a binary covariate and a constant covariate.

  3. aggregation="twfe" accepts covariate-adjusted CS fits. R's twfe_weights stops on three conditions: never-treated control, universal base period, and xformla == ~1. _guard_cs_design enforces the first two and its error text says it matches R, but nothing checks covariates because CallawaySantAnnaResults does not record them. A CS fit with covariates=[...] is accepted with no warning, and the result docstring then calls implied_att "the TWFE coefficient", which it is not. Record covariate usage on the result (the _aggregation_kit.bookkeeping dict is a reasonable home) and raise in the guard.

  4. The ATT(g,t)-frame route accepts incomplete, duplicated, and non-finite input. Reproduced: dropping the (g=3, t=3) post cell before attgt_weights(..., aggregation="overall") gives post weights summing to 0.8333 with no warning, so the result is no longer ATT^O; a duplicated (group, time) row is accepted silently; an inf ATT is accepted and yields implied_att = inf with no warning (the fitted-result path uses np.isfinite, the frame path only drops NaN). Please: reject duplicate cells; apply np.isfinite to att, group, and time; fail closed on an incomplete grid for twfe; and for overall/simple either raise when a required post cell is missing or define an available-cell estimand with explicit renormalization and document it. The NaN-cell warning text ("the reported weights renormalize") describes behaviour the code does not have for a dropped post cell.

  5. User weights are not validated. Only the total is checked. Negative weights (I passed -1 for the first 50 rows) are accepted and produce an implied_att; non-finite weights propagate. Require finite, non-negative weights with a positive total and positive treated and control mass, checked in one place that both entry points share.

  6. The 0/0-cell deviation affects more fields than REGISTRY admits, and those fields are never asserted. On sim_staggered the two cancelling cells sit on opposite sides of the pre/post split, so versus the pinned R values pretrend_bias and post_only differ by 1.2e-4, effective_sample_size by 0.99, and cell ess/remainder at those cells by more. All are user-visible (summary() prints them). The REGISTRY note should enumerate them with magnitudes, and test_twfe_weights_parity.py should assert pretrend_bias, post_only, effective_sample_size, cell ess, and cell remainder, tight where _degenerate_mask is empty and relaxed only where it fires. Related: the scalar-split relaxation currently fires for sim_staggered/fwl_nocov too, where the observed gap is 3e-15 - restrict it to fwl_gmin1; and fwl_gmin1 cell weights/ATTs are pinned but not asserted.

  7. Route the FWL linear algebra through the house helpers. _wls_coefficients uses np.linalg.lstsq, _drop_collinear a pivoted QR, and _demean_two_way its own alternating-projection loop. The library rule is that all estimation goes through diff_diff.linalg.solve_ols (with its rank-deficient handling and weights=) and diff_diff.utils.within_transform / demean_by_groups (which take weights= and a tolerance). Please replace the three with those; the parity suite will tell you immediately if anything moves.

  8. Test coverage for the decomposition half and the plot. decompose_twfe_weights and covariate_balance are exercised only in the golden-gated parity file; tests/test_twfe_weights.py covers attgt_weights alone, and plot_twfe_weights has no test at all. Of the raise/warn sites in the decomposition half, two are asserted anywhere. Please add a decomposition block to test_twfe_weights.py (the REGISTRY edge cases: unbalanced panel, no never-treated group, time-varying cohort, gmin1 with a first-period cohort, covariate_balance() without balance_covariates=, bad level=; plus estimate == decomposition + remainder and implied_att == estimate on a synthetic panel), a plot_twfe_weights test next to the plot_bacon tests (both kind= branches, ax= reuse, the balance branch raising when there is no balance table, and an all-NaN balance table producing a clear error rather than a nanmax warning), and regression tests for items 1-5.

  9. negative_weight_share counts pre-period cells. Over the g != 0 grid the TWFE weights sum to zero (post to +1, pre to -1), so n_negative > 0 and a share near 0.5 appear in every staggered design, including one with no negative post-period weight. The pathology the docstring describes is negative weight on post cells. Report post-only counts (or both, labelled), and add a REGISTRY Note defining the statistic - it has no R counterpart.

  10. Remove the AIPW residue. TWFEDecompositionResult docstrings describe method="aipw"; the R generator header says it pins implicit_aipw_weights and "two private two-period kernels" that have no Python surface; REGISTRY says the kernels are "pinned directly by the parity suite", but the test never reads two_period.*, decompose.aipw, or balance.aipw (roughly 2,100 unused floats). Either strip those blocks from the JSON or label them in meta and the header as reserved for the AIPW follow-up, and fix the prose. For that follow-up, note that the pinned AIPW golden is covariate-adjusted (a time-invariant covariate is not a no-op in the propensity score).

  11. Drop twfeweights_mpdta_panel.csv. It is benchmarks/data/mpdta_stata_panel.csv with renamed columns plus lpop_t = lpop * (period - 2002) / 5; the five shared columns are bit-identical. Have the parity fixture rename and derive from the existing file (the golden's columns map already makes the test column-name-agnostic), and record the mpdta provenance (data(mpdta, package="did")) in the JSON meta. The two simulated panels are fine to keep.

  12. Label-aligned cell comparisons. The decompose and balance parity tests compare weight/att by array position without asserting (group, time, post), and the JSON mixes positional labels (decompose.*.cells, balance.*.cells) with raw labels (attgt_weights.*). Map back to original labels in the generator so every block shares one convention, then assert labels the way test_weight_column does.

  13. Add the plotly backend to plot_twfe_weights. It is the only plot function in the module without backend="matplotlib"|"plotly"; its two file-mates plot_sensitivity and plot_bacon both carry it with a _render_*_mpl / _render_*_plotly split, so please follow that pattern (tests live in tests/test_visualization_plotly.py). Also type results as the result class rather than Any, and update the module docstring, which still lists only sensitivity and Bacon.

  14. Landing-page discipline. The README line is about 600 characters against 130-290 for its neighbours and carries two call signatures and an argument list; cut it to the sibling shape (method, one-clause description, upstream credit). The changelog fragment reads as a PR description; keep the headline bullet and the three sub-bullets and drop the naming rationale, golden paths, and bibliography, which live in REGISTRY and the API page.

  15. Weighted ECDF is quadratic in unique values. Each knot rescans the full vector, repeated per covariate, cohort, and period. Sort once and take cumulative normalized weights at unique-value boundaries.

  16. Annihilated-covariate Note. It says the rounding column "amplifies noise by ~1e16 and corrupts the weights". On mpdta's lpop the demeaned column is exactly unit-constant, in the FE span, and orthogonal to the treatment residual; keeping it changes the FWL residual by ~2e-18. Dropping an exactly-zero column is still right, so keep the behaviour but rewrite the Note as numerical hygiene. Also the 1e-10 relative threshold can drop a covariate with a large level and small genuine within-variation, and the "no within variation" warning would then be false.

  17. Weighted aggregation="twfe" is an extension R does not have (twfe_weights takes no w=). The algebra checks out on a balanced panel, but REGISTRY defines p_g unweighted; add a Note and a test against decompose_twfe_weights(weights=...).

  18. Constructed fixtures in tests/helpers/results_foundation.py. The ATTGTWeightsResult declares negative_weight_share=0.25 while its weights give 0.143, and the TWFEDecompositionResult fixture's post_only=0.5 does not match its cells. Derive them from the cells as estimate/decomposition already are.

  19. Two weak unit tests. test_implied_att_is_the_weighted_sum re-implements sum(w * att) and so cannot fail; the __all__ membership test checks three of the five exported names.

  20. Fixture prose. "A real pre-trend so pretrend_bias != 0" is not what the DGP does: x1 is iid and cohorts are assigned by unit index, so the 0.093 is sampling noise. Either make x1's mean depend on cohort or soften the claim. Likewise "well-conditioned by construction" for the covariate branch: both structured terms of xtv are absorbed by the two-way FE, leaving noise as the regressor.

  21. Docs housekeeping. docs/doc-deps.yaml: the new sources: block sits under the # BaconDecomposition banner above diff_diff/bacon.py; give it its own banner. llms-full.txt: every other plot function has a ### plot_x subsection; add one for plot_twfe_weights. benchmarks/R/requirements.R lacks twfeweights, BMisc, DRDID. Add docs/api/twfe_weights.rst to tests/test_doc_snippets.py::RST_FILES so its code blocks execute.

Please rebase onto current main when you push (it moved by one commit). Items 10-12 and 20 touch the R generator if you take them fully; everything else can be done against the committed goldens without re-running R.

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.

2 participants