Commit cc91a7d
Address PR #366 CI review round 1: scope TreatmentDoseShape as descriptive-only; fix WooldridgeDiD method kwarg
P1 (TreatmentDoseShape vs ContinuousDiD contract):
- Reviewer correctly flagged that the new `is_time_invariant` field
(per-unit non-zero distinct-count) does NOT match the actual
`ContinuousDiD.fit()` gate at `continuous_did.py:222-228`, which
uses `df.groupby(unit)[dose].nunique() > 1` over the FULL dose
column (including pre-treatment zeros). My nonzero-only check
silently classified `0,0,d,d` paths as time-invariant while
ContinuousDiD would reject them.
- Removed `is_time_invariant` field from `TreatmentDoseShape`
entirely. The pre-existing `PanelProfile.treatment_varies_within_unit`
field already encodes the correct ContinuousDiD prerequisite (matches
the estimator's nunique check at line 224) and is correctly documented
in §2 of the autonomous guide. Adding a second, narrower, mismatched
gate was confusing - the reviewer's "scope as descriptive-only" path
is the cleaner fix.
- Reframed `TreatmentDoseShape` docstring + autonomous guide §2
field reference: explicitly NOT a ContinuousDiD prerequisite.
`n_distinct_doses`, `has_zero_dose`, `dose_min/max/mean` provide
descriptive distributional context; `has_never_treated` (unit-level)
+ `treatment_varies_within_unit == False` (full-path constancy) +
`is_balanced` are the authoritative gates.
- Rewrote §5.2 worked example reasoning chain to use the existing
correct gates and added a counter-example showing
`has_zero_dose=True` does NOT imply `has_never_treated=True` (the
row-level vs unit-level distinction).
- Added `test_treatment_dose_does_not_gate_continuous_did` covering
the two contradictory cases the reviewer named: (1) `0,0,d,d`
within-unit dose path, asserting `treatment_varies_within_unit=True`
(the actual ContinuousDiD gate fires correctly); (2) row-level zeros
without never-treated units, asserting `has_zero_dose=True` BUT
`has_never_treated=False` (the two facts are distinct).
- Removed `test_treatment_dose_continuous_time_varying_within_unit`
and `test_treatment_dose_distinguishes_doses_at_high_precision` -
both tested the dropped `is_time_invariant` field.
P2 (WooldridgeDiD constructor kwarg):
- The autonomous guide §5.3 worked example used
`WooldridgeDiD(family="poisson")` but the actual constructor at
`wooldridge.py:264` takes `method=`. Following the example would
raise `TypeError: __init__() got an unexpected keyword argument
'family'`. Fixed in two places (the prose and the code snippet)
and added a negative assertion in `test_guides.py` to prevent
regression: `assert 'WooldridgeDiD(family="poisson")' not in text`.
CHANGELOG updated to reflect the revised TreatmentDoseShape scope.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>1 parent ce73058 commit cc91a7d
5 files changed
Lines changed: 146 additions & 117 deletions
File tree
- diff_diff
- guides
- tests
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
11 | | - | |
| 11 | + | |
12 | 12 | | |
13 | 13 | | |
14 | 14 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
197 | 197 | | |
198 | 198 | | |
199 | 199 | | |
200 | | - | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
201 | 207 | | |
202 | | - | |
| 208 | + | |
| 209 | + | |
203 | 210 | | |
204 | | - | |
205 | | - | |
206 | | - | |
207 | | - | |
208 | | - | |
209 | | - | |
210 | | - | |
211 | | - | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
212 | 221 | | |
213 | 222 | | |
214 | 223 | | |
| |||
334 | 343 | | |
335 | 344 | | |
336 | 345 | | |
337 | | - | |
338 | | - | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
| 349 | + | |
339 | 350 | | |
340 | 351 | | |
341 | 352 | | |
| |||
485 | 496 | | |
486 | 497 | | |
487 | 498 | | |
488 | | - | |
489 | | - | |
490 | | - | |
491 | | - | |
| 499 | + | |
| 500 | + | |
| 501 | + | |
| 502 | + | |
| 503 | + | |
| 504 | + | |
| 505 | + | |
| 506 | + | |
| 507 | + | |
492 | 508 | | |
493 | 509 | | |
494 | 510 | | |
| |||
700 | 716 | | |
701 | 717 | | |
702 | 718 | | |
703 | | - | |
704 | 719 | | |
705 | 720 | | |
706 | 721 | | |
| |||
714 | 729 | | |
715 | 730 | | |
716 | 731 | | |
717 | | - | |
718 | | - | |
719 | | - | |
720 | | - | |
721 | | - | |
722 | | - | |
723 | | - | |
724 | | - | |
725 | | - | |
726 | | - | |
727 | | - | |
728 | | - | |
| 732 | + | |
| 733 | + | |
| 734 | + | |
| 735 | + | |
| 736 | + | |
| 737 | + | |
| 738 | + | |
| 739 | + | |
| 740 | + | |
| 741 | + | |
| 742 | + | |
| 743 | + | |
| 744 | + | |
| 745 | + | |
| 746 | + | |
| 747 | + | |
| 748 | + | |
| 749 | + | |
| 750 | + | |
| 751 | + | |
| 752 | + | |
| 753 | + | |
| 754 | + | |
| 755 | + | |
| 756 | + | |
| 757 | + | |
| 758 | + | |
729 | 759 | | |
730 | 760 | | |
731 | 761 | | |
| |||
765 | 795 | | |
766 | 796 | | |
767 | 797 | | |
768 | | - | |
| 798 | + | |
769 | 799 | | |
770 | 800 | | |
771 | | - | |
| 801 | + | |
772 | 802 | | |
773 | 803 | | |
774 | 804 | | |
| |||
0 commit comments