-
-
Notifications
You must be signed in to change notification settings - Fork 22
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
Refactor cox regression functions to fix table structure #882
Conversation
Code Coverage Summary
Diff against main
Results for commit: a65d4a3 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is very good. I would only add some cross-links in the param explanations to bind it a bit more clearly to the fit_*. Maybe also a couple of simple examples for some variables, but up to you. The result is great though, as you manage to keep the old framework and fix the structure and naming issues! Thanks, @edelarua
Hi @Melkiades, One minor change - I added model caching in which decreases time to run I also added in a few more examples for the different |
No problem at all! This is a very good idea and if it is not there, it should be extended to other similar layout-generating s_* functions. This may solve other computation issues in other functions (fit_* family e.g.) I also did change the |
Closes #841
Downstream updates needed:
teal.modules.clinical
Updatetm_t_coxreg
teal.modules.clinical#756tlg-catalog
Update COXT01 and COXT02 tlg-catalog#36scda.test
Refactor COXT* tests scda.test#40