-
Notifications
You must be signed in to change notification settings - Fork 218
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
Cumulative Incidence (Kaplan-Meier) for competing risks. #491
Conversation
faed249
to
a65a142
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #491 +/- ##
==========================================
+ Coverage 98.21% 98.23% +0.02%
==========================================
Files 37 37
Lines 3521 3564 +43
Branches 464 472 +8
==========================================
+ Hits 3458 3501 +43
Misses 30 30
Partials 33 33 ☔ View full report in Codecov by Sentry. |
The codacy linter triggers for unused unpacked assignments, however the ruff linter doesn't. Perhaps it would be nice to make them consistent, though is not a big issue. (I can open a new issue/PR if needed, it is trivial to fix it from the ruff side). |
9b03384
to
477bdd0
Compare
477bdd0
to
1eb1435
Compare
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.
This is great, thanks a lot!
The code seems to be correct to me. I would suggest to improve the API docs a little bit to avoid ambiguities.
In addition, I'm not sure if certain corner cases are covered. For instance, what happens if no event has been observed for one (or more) competing risk?
I would appreciate if a test case with more than 2 competing risks could be added.
Thanks so much for the review! I also changed the function name (removing the reference to Kaplan-Meier). This is because I found a couple of papers that use KM for the naive extension of KM to the competing risks case, while we are using the rigorous estimator that does not assume independence between risks. |
bfa54f6
to
f06555f
Compare
f06555f
to
4c5e3ea
Compare
4f570c2
to
4ab1ed7
Compare
and its entry in datasets/data/README.md
4ab1ed7
to
3c975a9
Compare
Add non-parametric Cumulative Incidence (Kaplan-Meier) for competing risks. Tests included.
Checklist
What does this implement/fix? Explain your changes
Implements cumulative incidence estimator for the competing risks case. Confidence Intervals are not implemented but are under development.