GratefulPatientFeaturizer/EncounterTransformer: index-encounter keying vs. the PHI-free bundle contract need a design decision

Open
  • Is something wrong?
#110 1 comment 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
5/5
Estimated time
Over a week
Newbie friendliness
25/100
Issue type
Refactor
Clarity
Clearly specified
Activity status
Quiet
Tech stack
python, scikit-learn
Domain
data

Research direction

Read philanthropy/preprocessing/_grateful_patient.py, especially merge_key, last_discharge, and __getstate__, then review the PHI-free bundle contract in docs/explanation/compliance_considerations.md and the issue’s linked context in #78. The issue is closed by a written decision on the tradeoff; if retaining latest-discharge keying, document the recurrent-patient limitation in the compliance considerations file.

Written by the indexing model from the issue text.

Description

help wanted question roadmap
Status (2026-11-04)

Still undecided, and neither half of the 2026-09-06 proposal below has landed. Checked against e410de6: GratefulPatientFeaturizer.fit still keeps one last_discharge per donor (grouped[discharge_col].max() after the as_of cutoff), and no doc mentions the repeat-patient effect.

  • The "document it now" half is filed as #361 (docs only, no decision needed).
  • The per-donor sorted discharge-date array (option 2 as refined in that comment) is not filed. If it is accepted, it becomes its own feature issue with the searchsorted design, and this issue closes with the decision.
  • Relation to 1.0 (#351): GratefulPatientFeaturizer, EncounterTransformer and EncounterRecencyTransformer are all Tier 2 in docs/reference/index.md, which may change in a minor release with a Breaking note. So per-gift keying can ship in a 1.x minor; what 1.0 needs is the limitation documented (#361), not the redesign.

The one decision left for a person: adopt the sorted-date-array design as the plan (and file it), or keep latest-discharge keying permanently.

The conflict

GratefulPatientFeaturizer and EncounterTransformer key on the donor's
latest discharge (grep -n "merge_key" philanthropy/preprocessing/_grateful_patient.py,
groupby on merge_key then take last_discharge). For a donor with more than
one encounter, a gift is measured against whichever discharge happens to be
most recent at fit/transform time, not the discharge that was actually closest
in time to that specific gift. Recurrent patients (oncology, transplant) are
the people this most misrepresents.

The fix needs the raw per-gift, per-encounter rows available at transform
time so each gift can be joined to its own nearest prior discharge. That is
exactly what __getstate__ is designed to strip
(grep -n "__getstate__" -A8 philanthropy/preprocessing/_grateful_patient.py):
raw encounter_df is PHI-bearing and is replaced with None on serialisation
so a saved model bundle never carries patient rows (#78).

Per-gift keying and the PHI-free bundle contract are in direct conflict. This
is not a bug with an obvious patch; it needs a design decision on which
guarantee gives way, or a third option (e.g. storing only a de-identified
per-donor encounter-to-gift offset at fit time, if that is precise enough).

Why it matters now

This is the one item from the 2026-08-13 audit that landed as "documented
limitation" rather than "fixed" going into the JOSS submission. It affects the
grateful-patient timing story the paper's Software design section describes as
solved via as_of; per-gift keying is a separate, still-open gap in the same
story.

What would close this

A written decision (this issue, a discussion, or a design doc) on one of:

  1. Keep latest-discharge keying, document the recurrent-patient
    misattribution explicitly as an accepted limitation in
    docs/explanation/compliance_considerations.md.
  2. Store a de-identified per-donor "days since nearest prior discharge, as of
    each gift date" summary at fit time (no raw dates, no diagnosis), so
    __getstate__ still has nothing PHI-bearing to strip.
  3. Some other resolution.

This needs a person to decide the tradeoff; it cannot be closed by a PR alone.

Dominant language
Python
Stars
11
Forks
23
Avg merge
15h 13m
Merged PRs (30d)
30

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from PhilanthroPy-Project/PhilanthroPy

All issues in PhilanthroPy-Project/PhilanthroPy

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.