Skip to content

fix: reject missing wealth columns - #179

Merged
shivamlalakiya merged 2 commits into
PhilanthroPy-Project:mainfrom
be-student:codex/156-wealth-column-validation
Sep 6, 2026
Merged

fix: reject missing wealth columns#179
shivamlalakiya merged 2 commits into
PhilanthroPy-Project:mainfrom
be-student:codex/156-wealth-column-validation

Conversation

@be-student

Copy link
Copy Markdown
Contributor

What changed

An explicit wealth_cols list that matches no training column now raises an actionable ValueError naming requested and available columns. Partial matches and automatic detection remain valid. The dead post-validation DataFrame branch is removed, with feature-name behavior covered for frames and arrays.

Validation

  • focused WealthPercentileTransformer tests: 5 passed
  • make ci: 1,951 passed, 25 skipped, 97.53% coverage
  • mandatory pre-push hook: 1,951 passed, 25 skipped

Closes #156

Resolves the CONTRIBUTORS.md conflict by keeping both @be-student entries
(PhilanthroPy-Project#163 from PR PhilanthroPy-Project#180, PhilanthroPy-Project#156 from this PR).
@shivamlalakiya
shivamlalakiya merged commit 6dfafa5 into PhilanthroPy-Project:main Sep 6, 2026
15 checks passed
shivamlalakiya added a commit to be-student/PhilanthroPy that referenced this pull request Sep 6, 2026
shivamlalakiya added a commit that referenced this pull request Sep 6, 2026
…attempt (#187)

Two follow-ups after the #175/#176 duplicate-PR triage:

- CONTRIBUTING.md gets a "Claiming an issue" rule (comment before opening a
  PR, wait for maintainer assignment) so two people never again spend an
  evening fixing the same four-line bug. The issue-draft template states the
  same rule inline, so every newly filed good-first-issue carries it without
  depending on a reader following a link.

- @HeaTTap independently identified and fixed the same dead
  `hasattr(X, "columns")` branch as @Larslllllll, via #175. That PR was
  closed as subsumed by #179 (which carried the same fix), but @HeaTTap had
  no CONTRIBUTORS.md line as a result; Lars was already credited via #174.
  This adds it.
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.

WealthPercentileTransformer silently ignores a wealth_cols list that matches no column

2 participants