-
Notifications
You must be signed in to change notification settings - Fork 4
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
Document interfaces for custom longitudinal models #318
Conversation
Code Coverage Summary
Diff against main
Results for commit: fbfc8de Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Unit Tests Summary 1 files 42 suites 8m 11s ⏱️ Results for commit fbfc8de. ♻️ This comment has been updated with latest results. |
Unit Test Performance Difference
Additional test case details
Results for commit 991ced5 ♻️ 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.
Thanks @gowerc , I think it looks good, I just proposed a few wording changes. The question is if it would be helpful for the user to have a full example (in addition to the helpful snippets for each step) for how to extend with a custom longitudinal model.
@danielinteractive , I've attempted to address all the review comments. The big change here (based on your above comments) was that I've also added an additional Note that I also show how to fit a custom simulation function here though this isn't actually detailed anywhere, I've added a new issue (#323) to cover this in a separate PR. |
Closes #316
@danielinteractive - Would appreciate potentially a bit more scrutiny on this one as I imagine this is an area that users are likely to dabble in with 3rd party models that we haven't yet implemented. It is also quite a bit more complicated to explain than the survival models so would appreciate it if you could highlight areas that don't make sense or need further clarification.