Skip to content

chore: clean package static-analysis findings#611

Open
shawcharles wants to merge 2 commits into
igerber:mainfrom
shawcharles:chore/package-ruff-cleanup
Open

chore: clean package static-analysis findings#611
shawcharles wants to merge 2 commits into
igerber:mainfrom
shawcharles:chore/package-ruff-cleanup

Conversation

@shawcharles

@shawcharles shawcharles commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Closes #606.

Summary

  • declare the ResolvedSurveyDesign forward reference in bootstrap_utils.py under TYPE_CHECKING
  • remove no-placeholder f-strings from power.py
  • remove the stale invalid # noqa: WPS433 marker from spillover.py while preserving the deferred-import note
  • remove audited unused locals where the computed value has no downstream use
  • document intentionally omitted symmetric DRDID grid entries in staggered.py
  • add an Unreleased changelog entry

Scope

This intentionally does not touch:

  • public API re-exports / F401
  • TROP import ordering / E402
  • broad import sorting / I001
  • honest_did.py mathematical l names / E741
  • CI gates

Validation

Rebased onto current main at 7f8dfd10 and reran:

  • python -m black --check diff_diff/bootstrap_utils.py diff_diff/chaisemartin_dhaultfoeuille.py diff_diff/honest_did.py diff_diff/imputation.py diff_diff/power.py diff_diff/prep_dgp.py diff_diff/spillover.py diff_diff/staggered.py
  • python -m ruff check diff_diff --select F821,F541,F841,W --output-format=concise
  • python3.9 -m py_compile diff_diff/bootstrap_utils.py diff_diff/power.py diff_diff/spillover.py diff_diff/chaisemartin_dhaultfoeuille.py diff_diff/honest_did.py diff_diff/imputation.py diff_diff/prep_dgp.py diff_diff/staggered.py
  • git diff --check
  • PYTHONPATH=. DIFF_DIFF_BACKEND=python pytest tests/test_bootstrap_utils.py tests/test_bootstrap_chunking.py -q (94 passed)
  • PYTHONPATH=. DIFF_DIFF_BACKEND=python pytest tests/test_power.py tests/test_methodology_power.py tests/test_spillover.py tests/test_honest_did.py tests/test_methodology_honest_did.py tests/test_imputation.py tests/test_methodology_imputation.py tests/test_prep_dgp_reversible.py tests/test_staggered_rc.py tests/test_staggered.py tests/test_chaisemartin_dhaultfoeuille.py tests/test_dcdh_heterogeneity_cell_period_coverage.py -q (1443 passed, 104 deselected)

@shawcharles shawcharles marked this pull request as ready for review July 4, 2026 09:21
@shawcharles shawcharles force-pushed the chore/package-ruff-cleanup branch from 0a1a39a to db171a1 Compare July 11, 2026 12:39
@shawcharles

Copy link
Copy Markdown
Contributor Author

Rebased this onto current main (7f8dfd10) and reran the scoped validation. Black, the selected Ruff rules, Python 3.9 compilation, and git diff --check pass; the focused test groups pass with 94 and 1,443 tests respectively.

The requests from #606 remain addressed: the DRDID grid omissions are documented, the deferred-import note is preserved, and the Unreleased changelog entry is included. This is ready for review from my side. I do not have permission to apply ready-for-ci, so could you add that label when convenient?

@igerber

igerber commented Jul 11, 2026

Copy link
Copy Markdown
Owner

Thanks for the rebase - and apologies for the slow feedback loop on my end. I re-validated the rebased branch against current main: the selected Ruff rules are clean package-wide (and none of the ~60 PRs merged since your original branch introduced new findings), black --check passes on the touched files, mypy drops exactly the four ResolvedSurveyDesign name-defined errors with no new ones, and a full-precision comparison of CallawaySantAnna(estimation_method="dr", panel=False) with covariates - the code path where the mu0_*/ps_* locals were removed - is bit-identical between your branch and main: overall ATT, SEs, and every group-time effect. The behavior-neutrality claim is confirmed empirically, not just statically.

Two changes before this can go in:

  1. CHANGELOG section: your entry is currently inside the released ## [3.6.2] - 2026-07-03 section (around line 957). Since then 3.7.0 has also shipped, and there's currently no [Unreleased] heading in the file. Please add ## [Unreleased] at the top (above ## [3.7.0] - 2026-07-08) and move your entry there - released sections shouldn't change retroactively.

  2. Propensity comment wording (minor): the new comment says the treated slices aren't materialized because "only control propensity slices feed IPW control weights". That's true for the weights, but treated propensity scores do still enter the influence function via the full pscore array in the propensity-estimation correction term (score_ps = (D_all - pscore)[:, None] * X_all_int). To avoid a future reader concluding treated pscores are unused entirely, please append something like: "(the propensity-estimation IF correction consumes the full pscore array directly)".

Everything else looks ready. Once you've pushed those revisions I'll add the ready-for-ci label to kick off CI, and we should be good to go.

@shawcharles

Copy link
Copy Markdown
Contributor Author

Addressed both requested revisions in 50645f37:

  • added ## [Unreleased] above 3.7.0 and moved the internal cleanup entry out of released 3.6.2;
  • clarified that only control slices feed the IPW control weights while the propensity-estimation IF correction consumes the full pscore array.

Validation after the revisions: package-wide selected Ruff rules pass, Black and Python 3.9 compilation pass, git diff --check passes, and tests/test_staggered_rc.py tests/test_staggered.py pass (261 passed). Ready for ready-for-ci.

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.

Clean package-source Ruff static-analysis findings

2 participants