Conversation
mbannick
left a comment
There was a problem hiding this comment.
Hi @dbackenroth, thanks for submitting this PR! Could you please add some brief documentation for this in the function?
|
Hi @mbannick I added in some documentation, please let me know what else I can provide. |
R/adjust-logrank.R
Outdated
| se <- sqrt(var_CSL) | ||
| statistic <- U_CSL / se | ||
|
|
||
| if ("id" %in% colnames(df)) { |
There was a problem hiding this comment.
@dbackenroth Thanks for adding documentation explaining this PR!
If someone has a covariate that is named id (that would be a strange name for a covariate, but it's possible, particularly if they are doing stratification by some type of id), then this may produce unintended behavior. I don't want anything to break for current users since this is a stable package.
Could you make the following change please:
- Modify
robincar_logrankso that it takes areturn_influenceargument that is by default FALSE, and then propagate that argument tomodeland then it can be accessed here and below, asif model$return_influencerather thanif "id" %in% ...? This way, users won't see any change in behavior based on default usage.
There was a problem hiding this comment.
@dbackenroth it also looks like some tests fail with the current PR. Could you please ensure the test suite runs through successfully? Thanks! See e.g., https://github.com/mbannick/RobinCar/actions/runs/20396466576/job/58744492353?pr=37.
There was a problem hiding this comment.
@mbannick hi, i added in unit tests. two of the tests are failing for me but I don't think that has to do with my code (test-glm-calibration.R: test calibration and test-glm-legacy.R: GLM legacy), it's related to functions not being found, e.g., str_detect. I added in two columns to robincar_tte signature, one is return_influence and the other is id_col. i may have gone overboard with the error checking, please let me know if you'd like me to make any further changes
calculates influence function for CALRT and returns to user (if user includes an "id" column in input dataset)