Skip to content

Add na_action handling to poststratify#260

Closed
neuralsorcerer wants to merge 7 commits intofacebookresearch:mainfrom
neuralsorcerer:post
Closed

Add na_action handling to poststratify#260
neuralsorcerer wants to merge 7 commits intofacebookresearch:mainfrom
neuralsorcerer:post

Conversation

@neuralsorcerer
Copy link
Collaborator

Added na_action handling to poststratify so missing values can be dropped or treated as their own category during weighting. Also, added tests covering na_action behaviors and validation in poststratify.

Copilot AI review requested due to automatic review settings January 13, 2026 06:06
@meta-cla meta-cla bot added the cla signed label Jan 13, 2026
@neuralsorcerer neuralsorcerer added this to the balance 0.15.0 milestone Jan 13, 2026
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds na_action parameter to the poststratify function to provide explicit control over missing value handling during post-stratification weighting. The implementation follows the same pattern as the rake method.

Changes:

  • Added na_action parameter with "add_indicator" (default) and "drop" options
  • Implemented NA handling after transformations, consistent with rake
  • Added tests for both na_action behaviors and parameter validation
  • Updated CHANGELOG with user-facing documentation

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
balance/weighting_methods/poststratify.py Added na_action parameter with validation and implementation for both "drop" and "add_indicator" modes; removed TODO comment about adding na_action
tests/test_poststratify.py Added test_poststratify_na_action() covering basic functionality and validation test for invalid na_action values
CHANGELOG.md Documented new na_action parameter in New Features section

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

Copy link
Contributor

@talgalili talgalili left a comment

Choose a reason for hiding this comment

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

Please fix failing tests :)

Copilot AI review requested due to automatic review settings January 13, 2026 08:46
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Copy link
Contributor

@talgalili talgalili left a comment

Choose a reason for hiding this comment

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

ok, I'm sorry, I take it back.
Adding bool as an option seems cumbersome.
Could you please just revert back to na_action as string - with the options you had implemented?

(sorry for the back and forth)

@neuralsorcerer
Copy link
Collaborator Author

ok, I'm sorry, I take it back. Adding bool as an option seems cumbersome. Could you please just revert back to na_action as string - with the options you had implemented?

(sorry for the back and forth)

no problem. will do it :)

Copilot AI review requested due to automatic review settings January 13, 2026 09:11
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

@talgalili
Copy link
Contributor

@neuralsorcerer could you please fix the copilot's comments?

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Copy link
Contributor

@talgalili talgalili left a comment

Choose a reason for hiding this comment

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

LGTM!

@meta-codesync
Copy link

meta-codesync bot commented Jan 13, 2026

@talgalili has imported this pull request. If you are a Meta employee, you can view this in D90583549.

@meta-codesync
Copy link

meta-codesync bot commented Jan 13, 2026

@talgalili merged this pull request in 06e72cf.

@neuralsorcerer neuralsorcerer deleted the post branch January 13, 2026 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants