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

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

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

No branches or pull requests

2 participants