Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

documentation of summary.coxph.penal()$coefficients and consistency with summary.coxph()$coefficients #293

Open
ThomasSoeiro opened this issue Jan 16, 2025 · 2 comments

Comments

@ThomasSoeiro
Copy link

ThomasSoeiro commented Jan 16, 2025

?summary.coxph says:

Value

An object of class summary.coxph, with components:
(...)
coefficients   a matrix with one row for each coefficient, and columns containing the coefficient, the hazard ratio exp(coef), standard error, Wald statistic, and P value.
(...)

And as documented:

> fit <- coxph(Surv(time, status) ~ age + sex, lung) 
> summary(fit)$coefficients
           coef exp(coef)    se(coef)         z    Pr(>|z|)
age  0.01704533  1.017191 0.009223273  1.848078 0.064591012
sex -0.51321852  0.598566 0.167457962 -3.064760 0.002178445

But there is no ?summary.coxph.penal (it "redirects" to ?coxph) and the coefficients matrix is different:

> fit <- coxph(Surv(time, status) ~ pspline(age) + sex, lung) 
> summary(fit)$coefficients
                            coef    se(coef)         se2    Chisq       DF
pspline(age), linear  0.01689968 0.009032792 0.009032362 3.500361 1.000000
pspline(age), nonlin          NA          NA          NA 2.940696 3.092186
sex                  -0.51821169 0.168425387 0.168126761 9.466715 1.000000
                               p
pspline(age), linear 0.061355442
pspline(age), nonlin 0.416841834
sex                  0.002092337

It would be useful to document summary.coxph.penal()$coefficients (and other possible differences with summary.coxph()) and add exp(coef) in the matrix.

@therneau
Copy link
Owner

The print function for coxph.penal objects with a pspline or frailty term is special, but summary is, by design, the same for the two of them. The underlying definition of print (original Statistical Models in S book) is a 'short' and summary a 'longer' printout of the fit, and I am consistent with that.

What I suspect you are actually asking for is for summary.coxph to include another element, which contains the shorter output of print? The summary.survfit function does this: it includes a "table" component with the results shown in print.survfit. It's a reasonable request, and I'll add it to the list. But be forewarned that the request list is long.

I need to learn a way to copy a pull request down to my test machine, create a test copy of survival, and run R CMD check, BEFORE accepting any changes. You would be surprised to learn how often innocuous changes have unforseen consequences in one of the 890 reverse dependencies.

@ThomasSoeiro ThomasSoeiro changed the title documentation of summary.coxph.penal()$coefficient and consistency with summary.coxph()$coefficient documentation of summary.coxph.penal()$coefficients and consistency with summary.coxph()$coefficients Jan 24, 2025
@ThomasSoeiro
Copy link
Author

ThomasSoeiro commented Jan 24, 2025

Sorry, my first post was misleading because there was a typo (now fixed): the commands in the examples were not summary(fit) but summary(fit)$coefficients. I also clarify what is currently documented.

The print function for coxph.penal objects with a pspline or frailty term is special, but summary is, by design, the same for the two of them. The underlying definition of print (original Statistical Models in S book) is a 'short' and summary a 'longer' printout of the fit, and I am consistent with that.

My point is that both summary() methods do not return exactly the same, in particular for the coefficients component.

What I suspect you are actually asking for is for summary.coxph to include another element, which contains the shorter output of print? The summary.survfit function does this: it includes a "table" component with the results shown in print.survfit. It's a reasonable request, and I'll add it to the list. But be forewarned that the request list is long.

No, this is not what I meant. I was suggesting:

  • to document summary.coxph.penal() (which currently "redirects" to ?coxph), in particular the content of summary.coxph.penal()$coefficients;
  • and to add exp(coef) in the matrix in summary.coxph.penal()$coefficients, to match summary.coxph()$coefficients.

I need to learn a way to copy a pull request down to my test machine, create a test copy of survival, and run R CMD check, BEFORE accepting any changes. You would be surprised to learn how often innocuous changes have unforseen consequences in one of the 890 reverse dependencies.

Does it help? https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally#modifying-an-inactive-pull-request-locally

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants