Skip to content

Conversation

@jrwishart
Copy link
Contributor

@jrwishart jrwishart commented Nov 17, 2025

Update the handling of missing data for weighted regression models and the calculation of probabilities. The missing values should remain by default but are configurable otherwise.

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 addresses handling of missing data in survey-weighted logistic regression models by making the na.action parameter configurable in the Probabilities.Regression function.

Key Changes:

  • Modified Probabilities.Regression to accept and use an optional na.action parameter from the ... arguments, defaulting to na.pass if not provided
  • Added an na.action attribute to the validated newdata to ensure NA rows are preserved for survey models
  • Updated Binary Logit predictions to use the configurable na.action instead of the hardcoded na.pass

Reviewed Changes

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

File Description
R/variables.R Modified Probabilities.Regression to extract and use an optional na.action parameter, allowing callers to control missing data handling (e.g., na.omit vs na.pass) for Binary Logit predictions
tests/testthat/test-dataproblems.R Added test verifying that survey-weighted Binary Logit models preserve all respondents (100 rows) when using default na.pass, and correctly filter to only complete cases (90 rows) when explicitly passing na.action = na.omit
Comments suppressed due to low confidence (1)

R/variables.R:225

  • The na.action parameter is now configurable for Binary Logit models, but Ordered Logit and Multinomial Logit models on line 225 still use the hardcoded na.pass. For consistency, these model types should also respect the na.action parameter from the function arguments.

Consider changing:

probs <- suppressWarnings(predict(object$original, newdata = newdata,
                                  na.action = na.pass, type = "probs"))

to:

probs <- suppressWarnings(predict(object$original, newdata = newdata,
                                  na.action = na.action, type = "probs"))
        probs <- suppressWarnings(predict(object$original, newdata = newdata,
                                          na.action = na.pass, type = "probs"))

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@JustinCCYap JustinCCYap left a comment

Choose a reason for hiding this comment

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

LGTM

@jrwishart jrwishart merged commit 3ee0917 into master Nov 17, 2025
1 of 2 checks passed
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.

3 participants