Skip to content

feat: add S-Learner, T-Learner, and X-Learner implementations with ut…#39

Merged
rmorton8 merged 3 commits into
mainfrom
feature/uplift-metalearners
Jul 8, 2026
Merged

feat: add S-Learner, T-Learner, and X-Learner implementations with ut…#39
rmorton8 merged 3 commits into
mainfrom
feature/uplift-metalearners

Conversation

@rmorton8

@rmorton8 rmorton8 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

…ility functions

Also -- make sure sklearn indexes SliceDict with pandas indexing if values are pandas otherwise non-default index will cause unexpected behavior

@jwdink -- This branch was created based on the feature/uplift branch which included too many older commits that appeared unrelated to uplift modeling. See draft PR 38

…ility functions

Also -- make sure sklearn indexes SliceDict with pandas indexing if values are pandas
otherwise non-default index will cause unexpected behavior
@rmorton8
rmorton8 requested a review from jwdink July 7, 2026 16:29
@rmorton8 rmorton8 self-assigned this Jul 7, 2026

@jwdink jwdink left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you bump the version and do a quick addition of type-hints where it's easy to do so? then g2g

Comment thread foundry/uplift/meta_learners/s_learner.py

return yhat_t - yhat_c

def score(self, X, y, sample_weight=None, method='qini', normalize=True, **kwargs) -> float:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add type-hints?

@rmorton8 rmorton8 Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done. TIL about @overload

@rmorton8
rmorton8 requested a review from jwdink July 8, 2026 15:17
Comment thread foundry/util.py Outdated
Co-authored-by: Jacob <6982295+jwdink@users.noreply.github.com>
@rmorton8
rmorton8 merged commit 192c27f into main Jul 8, 2026
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