Closed loffleraSMH closed 3 months ago
Thanks so much for the review and the helpful feedback @guoyi-smh! I fixed the bug and implemented your other suggestions. Let me know if there are any other changes you'd like me to make.
Hi @loffleraSMH , all the updates look good. I have resolved all the comments. Please feel free to go ahead and merge the branch.
Closes #76
Main changes
utils.R
that's calledconvert_dt
: It converts any date-time variables into the format expected by other Rgemini functions (typically: “ymd HM”, which is used as the default format byconvert_dt
) and returns warning messages informing users about invalid/missing date-times and (if relevant) about entries with missing timestamps/timestamps that are 00:00ymd_hm
calls withconvert_dt
(even in cases where it’s not technically needed, but just to standardize the warning messages about invalid/missing date-times)lubridate
imports where they are not needed anymore@params
sections (where necessary) to indicate acceptable formats (character or POSIXct POSIXt)length_of_stay()
,er_los()
,day_time_of_admission()
,hospital_fiscal_year()
,stat_holidays_ON()
,episodes_of_care()
,readmission()
,daily_census()
,icu_entry()
,icu_los()
,mlaps()
n_routine_bloodwork()
,n_imaging()
, andn_rbc_transfusions()
I haven’t made any changes yet to avoid merge conflicts with Yishan’s PR for issue #125. I will open a separate issue for those functions because a lot of the date-times are currently handled within the SQL query (so we’d need to refactor those functions first before we can applyconvet_dt()
).For a few functions, I made some additional changes:
er_los()
: As discussed with Yishan, I made a small change to this function to return genc_ids with an entry in er but missing/invalid ER date-time aser_los = NA
(instead ofer_los = 0
). This only has a minimal impact on the function output (<1% of all entries in the er table are affected by this). I’ve also added a warning about this in the function and updated the documentation accordingly.icu_los()
: Similar toer_los
, any genc_ids with a missing/invalid date-time in ipscu are now returned asicu_los = NA
.icu_entry()
: I discussed this with Bijun, for any time-sensitive ICU flags (e.g., ICU within 24h), we now return genc_ids with 0 valid ICU admission date-times as NA. For any genc_ids with at least 1 valid date-time, any valid entries are considered in the calculation. I’ve updated the documentation accordingly.day_time_of_admission()
: I switched the code to standard lubridate functions (instead of str operations) to allow for more flexible date-time formats and to make sure timestamps with 00:00 are handled appropriately (the previous version returned any entries with 00:00 as NA for daytime). Otherwise, the output of the current versions is the same as before.Here is an example for how to test the code for
length_of_stay()
: