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

Add semi parametric survival analysis (Cox proportional hazards) #5

Merged
merged 24 commits into from
Oct 28, 2024

Conversation

FBartos
Copy link
Collaborator

@FBartos FBartos commented Oct 8, 2024

adds semi-parametric survival analysis
Should the analysis be called "Cox proportional hazards" or "Semiparametric"
(later we will add "parametric models" (under a single umbrella because they can have many different likelihoods, similar to glms)
links: jasp-stats/jasp-issues#206 (does not add Bayesian versions)
the proper qml display requires commits pos 19.1

plotting completely switched from survminer to ggsurvfit due to a) patchwork and better flexibility

some nice data sets for testing:
Rossi.csv
Rossi.long.csv --- can be used for the counting input
bladder.csv

@FBartos FBartos requested a review from juliuspfadt October 8, 2024 12:45
@FBartos
Copy link
Collaborator Author

FBartos commented Oct 10, 2024

@boutinb there are couple of qml bugs with the height of VariablesForm
could you check SemiParametricSurvivalAnalysis.qml the for // TODO: Bruno

@juliuspfadt
Copy link
Contributor

why can this PR actually be merged without my approval? Shouldn't there be securities that there needs to be some contingency?

@juliuspfadt
Copy link
Contributor

I think the name "semiparametric" sounds good

@juliuspfadt
Copy link
Contributor

juliuspfadt commented Oct 13, 2024

So when I choose Counting with the Rossi.csv data, the GUI fucks up. Same with the RossiLong data

@juliuspfadt
Copy link
Contributor

When Method=Exact and some variable is put in Cluster, I get an not very informative error:
error1.jasp.zip

@juliuspfadt
Copy link
Contributor

Another error, the tables are all empty for the specific combination of options:
Screenshot 2024-10-13 at 20 38 04
error2.jasp.zip
I assume that is because theta is by default 0 (same for Df by the way) and likely makes no sense. If you think people will know for sure that this is the reason and they will not look somewhere else what could have caused the empty tables, then I think it is fine. If there is the slightest doubt and users are as stupid as me, you should at least add a footnote beneath a table.

@juliuspfadt
Copy link
Contributor

the hazard estimation table shows weird numbers for this
error3.jasp.zip

@juliuspfadt
Copy link
Contributor

I dont get why the Plot has Options Legend and Color Palette. For me, I do not see a legend nor colors. It's a grey plot. Maybe I am misunderstanding something.

@juliuspfadt
Copy link
Contributor

juliuspfadt commented Oct 13, 2024

I get some plot errors with this:
error3.jasp.zip
Happens for residuals plots type Score, Schoenfeld, Schoenfeld scaled

@juliuspfadt
Copy link
Contributor

where are the tests? and where are the help files?

Copy link
Contributor

@juliuspfadt juliuspfadt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quite the big PR, nice work, especially coding the conditions in qml as a function. I am going to steal that!

inst/qml/qml_components/SurvivalPlot.qml Outdated Show resolved Hide resolved
inst/qml/SemiParametricSurvivalAnalysis.qml Outdated Show resolved Hide resolved
R/nonparametricsurvivalanalysis.R Show resolved Hide resolved
R/semiparametricsurvivalanalysis.R Show resolved Hide resolved
R/semiparametricsurvivalanalysis.R Outdated Show resolved Hide resolved
R/semiparametricsurvivalanalysis.R Show resolved Hide resolved
R/semiparametricsurvivalanalysis.R Show resolved Hide resolved
R/semiparametricsurvivalanalysis.R Outdated Show resolved Hide resolved
R/semiparametricsurvivalanalysis.R Outdated Show resolved Hide resolved
R/semiparametricsurvivalanalysis.R Outdated Show resolved Hide resolved
@FBartos
Copy link
Collaborator Author

FBartos commented Oct 21, 2024

all but the UI is fixed now

@FBartos FBartos requested a review from juliuspfadt October 21, 2024 13:44
@juliuspfadt
Copy link
Contributor

the hazard estimation table shows weird numbers for this error3.jasp.zip

did you check all these errors. Cause this one seems to be still there for me.

Copy link
Contributor

@juliuspfadt juliuspfadt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

most of it seems fine, but not all :)

R/commonsurvivalanalysis.R Outdated Show resolved Hide resolved
R/commonsurvivalanalysis.R Outdated Show resolved Hide resolved
R/semiparametricsurvivalanalysis.R Show resolved Hide resolved
R/semiparametricsurvivalanalysis.R Outdated Show resolved Hide resolved
inst/Description.qml Outdated Show resolved Hide resolved
inst/qml/SemiParametricSurvivalAnalysis.qml Outdated Show resolved Hide resolved
@FBartos
Copy link
Collaborator Author

FBartos commented Oct 23, 2024

fixed the remaining issues

@FBartos FBartos requested a review from juliuspfadt October 25, 2024 12:47
Copy link
Contributor

@juliuspfadt juliuspfadt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should start working with the info stuff in qml, nice.
The last error is fixed for me. Not sure about the description file, did ou update the version? Although, most of my modules are a bit behind either...

@FBartos FBartos force-pushed the add-semi-parametric branch from 938e337 to 5793f9f Compare October 28, 2024 13:42
@FBartos FBartos merged commit 1945277 into jasp-stats:master Oct 28, 2024
1 check passed
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

Successfully merging this pull request may close these issues.

3 participants