Skip to content

Clean up and address top tech debt#37

Merged
igerber merged 2 commits intomainfrom
claude/tech-debt-review-4Lj04
Jan 4, 2026
Merged

Clean up and address top tech debt#37
igerber merged 2 commits intomainfrom
claude/tech-debt-review-4Lj04

Conversation

@igerber
Copy link
Copy Markdown
Owner

@igerber igerber commented Jan 4, 2026

Summary of Changes

  • Ruff linter errors
  • 10 mypy type errors
  • Duplicate function removal
  • Bare exception handling
  • Document cluster default
  • API consistency

claude added 2 commits January 4, 2026 20:09
- Add Priority Items section with linter/type errors and quick wins
- Document 2 ruff errors (unused import, unsorted imports)
- Document 10 mypy errors in staggered.py (Optional type handling)
- Add bare exception handling locations with recommendations
- Document code duplication across modules
- Add API inconsistency notes for bootstrap parameters
- Update module line counts and splitting recommendations
- Add note about expected visualization test skips
Tier 1 - Blocking fixes:
- Fix ruff linter errors (unused Union import, unsorted imports)
- Fix 10 mypy type errors in staggered.py (Optional dict access guards)
- Remove duplicate _get_significance_stars() from diagnostics.py

Tier 2 - Quality improvements:
- Replace bare `except Exception` with specific exceptions:
  - diagnostics.py: ValueError, KeyError, LinAlgError
  - honest_did.py: ValueError, TypeError
- Document TwoWayFixedEffects cluster default behavior in docstring
- Add bootstrap_weights parameter to CallawaySantAnna with deprecation
  warning for bootstrap_weight_type (will be removed in v2.0)

Test updates:
- Update test_staggered.py to test new bootstrap_weights parameter
- Ensure backward compatibility with deprecated parameter

All tests pass (456 passed, 21 skipped).
@igerber igerber merged commit 98507da into main Jan 4, 2026
1 check passed
@igerber igerber deleted the claude/tech-debt-review-4Lj04 branch January 4, 2026 21:54
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