Added the combination-window function #63
Conversation
Jay-sanjay
commented
Aug 27, 2024
•
Jay-sanjay
commented
- Added the actual function
- Added the test suite
- Added the doc-strings
- Added the example in the doc-strings
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #63 +/- ##
===========================================
- Coverage 100.00% 96.73% -3.27%
===========================================
Files 2 2
Lines 49 92 +43
===========================================
+ Hits 49 89 +40
- Misses 0 3 +3 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Nice work @Jay-sanjay ! This is one of the trickier parts of the implementation so I added a bunch of clarifying questions as I had a little bit of trouble following your implementations. Additionally, could you add docstrings to your functions here? I think that would be helpful to know what these functions are doing.
Thanks!
src/preprocessing.jl
Outdated
| overlap = shift(treatment_history.event_end_date, 1) .- treatment_history.event_start_date | ||
| # Correctly handle Missing values before calling Dates.value | ||
| overlap_days = [ismissing(o) ? 0 : Dates.value(o) for o in overlap] | ||
| selected_rows = findall(treatment_history.SELECTED_ROWS .== 1) |
There was a problem hiding this comment.
Could you explain to me exactly what these lines are doing? Could you give me as much details as possible and how this relates to the original implementation?
| append!(treatment_history, DataFrame(new_row')) | ||
| else | ||
| # LRFS | ||
| treatment_history[i, :event_end_date] = treatment_history[i-1, :event_end_date] |
There was a problem hiding this comment.
I am not entirely sure this is right looking at the Reference R Code here: https://github.com/mi-erasmusmc/TreatmentPatterns/blob/171b60af7e76f63e09f2c2d48e2b811ce1dafff0/R/ConstructPathways.R#L350
Did you just dramatically simplify the implementation here? Or what did you do? I am having a little difficulty following your implementation versus the R version.
There was a problem hiding this comment.
Hey @Jay-sanjay , I fixed things up here and this looks good to me to merge. Go for it!
