Skip to content
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

ggcoxdiagnostics() is not working properly for schoenfeld residuals #122

Closed
pbiecek opened this issue Jan 23, 2017 · 1 comment
Closed

ggcoxdiagnostics() is not working properly for schoenfeld residuals #122

pbiecek opened this issue Jan 23, 2017 · 1 comment

Comments

@pbiecek
Copy link
Contributor

@pbiecek pbiecek commented Jan 23, 2017

Some residuals (like schoenfeld) are calculated for each failure time, not each observation.

Thus lines 91-94 will trigger an error because a data frame with schoenfeld residuals (n.rows = number of failure times) cannot be bind to vector of linear predictions for observations (=number of rows in the data).

The easiest fix is to replace xval <- 1:fit$n by xval <- 1:nrow(res)
and change the default value for linear.predictions e.g. by linear.predictions = type %in% c("martingale", "score", "dfbeta", ...)

Expected behaviour

Example for ggcoxdiagnostics() will work also for schoenfeld residuals

coxph.fit2 <- coxph(Surv(futime, fustat) ~ age + ecog.ps, data=ovarian)
ggcoxdiagnostics(coxph.fit2, type = "schoenfeld")

Actual behaviour

Error in `$<-.data.frame`(`*tmp*`, "xval", value = c(2.60222487953181,  : 
  replacement has 26 rows, data has 12

Steps to reproduce the problem

coxph.fit2 <- coxph(Surv(futime, fustat) ~ age + ecog.ps, data=ovarian)
ggcoxdiagnostics(coxph.fit2, type = "schoenfeld")
@pbiecek pbiecek changed the title ggcoxdiagnostics() is now working properly for schoenfeld residuals ggcoxdiagnostics() is not working properly for schoenfeld residuals Jan 24, 2017
pbiecek added a commit to pbiecek/survminer that referenced this issue Jan 25, 2017
kassambara added a commit that referenced this issue Jan 25, 2017
fix for #122
@pbiecek pbiecek closed this Jan 25, 2017
kassambara added a commit that referenced this issue Jan 25, 2017
@kassambara
Copy link
Owner

@kassambara kassambara commented Jan 25, 2017

Bug fixed now by the pull request #123. Thank you for pointing this out and for your work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.