Skip to content

fix: accept parsed gift dates - #180

Merged
shivamlalakiya merged 1 commit into
PhilanthroPy-Project:mainfrom
be-student:codex/163-parsed-gift-dates
Sep 6, 2026
Merged

fix: accept parsed gift dates#180
shivamlalakiya merged 1 commit into
PhilanthroPy-Project:mainfrom
be-student:codex/163-parsed-gift-dates

Conversation

@be-student

Copy link
Copy Markdown
Contributor

Fixes #163.

Normalizes the gift-date column before scikit-learn validates the mixed-dtype frame, so both strings and datetime64 inputs work. Invalid non-missing dates now raise a column-specific error.

Validation: make ci (1,951 passed, 25 skipped; 97.44% coverage).

@shivamlalakiya
shivamlalakiya merged commit 828639f 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
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 added a commit to be-student/PhilanthroPy that referenced this pull request Sep 6, 2026
@shivamlalakiya

Copy link
Copy Markdown
Contributor

@be-student three merged fixes in one day, and all three were real defects rather than coverage padding. #179 in particular closed a silent no-op: a wealth_cols list that matched no column was accepted and quietly ignored, so a user would have got numbers back with no indication that the transformer had done nothing. That's the failure mode I most want caught, because nothing downstream ever raises.

You clearly read the codebase rather than just the issue text, so I'd rather point you at something open-ended than another labelled issue.

#110 is the one unresolved design question in the hospital-fundraising path: index-encounter keying versus the PHI-free bundle contract, which tests/test_no_phi_in_bundles.py currently depends on. I've written up why it's deferred rather than decided, but a deferral I argued with myself about is worth less than one someone else has pushed back on. If you have a view, the thread is the right place for it.

Separately, if anything about getting set up here was annoying, I'd like to hear that too: #188

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.

EncounterTransformer rejects a parsed datetime64 gift_date and accepts only date strings

2 participants