New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chill out, Background #415
Comments
I'm not convinced we want to automatically
The two options that seem reasonable to me are
Personally I like the first. |
I agree 1 is the ideal solution. This would also require a review of existing models to ensure they work with |
But the current models don't need to work with NA values. I guess it would be good if they could error in a useful way if they can't handle NAs. Maybe that's what you mean by "work with NA values". But the workflow I imagine is:
|
Yes, I mean throw a sensible error or remove NAs with warning |
OK yes then I totally agree. Should throw a sensible error. |
Yeah, I agree. Best thing is to make the existing models remove NAs (with a warning) so we don't break any existing workflows,and to remove na.omit here. |
Just to go back to this the |
Background
has anna.omit()
step before returning the dataframe, which removes rows if any column has NA values.Ideally, this should only remove rows with NAs in the the covariate columns (those columns named in the
covCols
attribute), since otherwise it causes problems if there are user-defined columns (#414).The text was updated successfully, but these errors were encountered: