15 ed exclusion flag for n imaging and n bloodwork

[guoyi-smh]

closes #15

Refactored n_imaging, n_routine_bloodwork and n_rbc_transfusion. Now all functions take three arguments:

• dbcon: database connection
• cohort: patient list
• exclude_ed: a new logical argument to indicate whether tests in ED to be excluded.

Other changes include:

• All three functions will now print a note indicating that the function currently does not take data coverage into account and hence it is the user's responsibility to check data availability and interpret the 0s in the output table.
• When exclude_ed is set to FALSE, all tests are included, including tests without date time, tests before triage and after discharge
• n_imaging will no longer take a user specified date-time variable name, following implementation in MPR, ordered_date_time is used, when it is not available, performed_date_time is used instead to identify tests in ED.
• n_imaging will not take a user-specified modality variable name, it will automatically use modality_mapped from GEMINI radiology table.
• For n_rbc_transfusion, following implementation in MPR, transfusions without a valid hemoglobin test in previous 48h are not counted.
• For n_routine_bloodwork, the filter to include only numeric results is now removed, to be consistent with definition in MPR and OPR. The rationale is to count all tests where blood was drawn from patients regardless of result values.
• check_input is now applied to all input arguments

[loffleraSMH]

@guoyi-smh one more general comment/question (for a future sprint): Some of the filtering by date-time variables happens within the SQL query now, which probably makes it more efficient, but with this approach it’s not obvious how many date-time variables are missing/invalid. I’ve been adding a new utility function that transforms/checks date-time variables (see issue #76), so I think eventually, I’d like to apply this within these 3 functions as well so users are informed about missing/invalid date-times? This might be particularly relevant for imaging, so we can inform users how missing order date-times are handled. That would mean we’d query all entries first and then perform the filtering by date-times after. Any thoughts on this approach? If you agree, I can open a new issue for that so we can tackle that in a future update.

[guoyi-smh]

Thanks so much for implementing this Yishan! It’s nice to have this improved consistency across the 3 functions and also have the additional warning messages.

I tested all functions to make sure they run as expected. For n_rbc_transfusions, the new output is a tiny bit different from before (when exclude_ed = TRUE) but I believe that’s simply due to the fact that the previous version excluded transfusions after discharge? For n_routine_bloodwork and n_imaging, the results are identical (if exclude_ed = FALSE) as expected.

I can do one more quick round of review once you’ve had a chance to address the comments above (no rush!).

I think you are right, the difference in transfusion should be caused by the filter on discharge_date_time. I dropped that for now to be consistent across 3 function. I had a conversation with Surain on this, and he suggested that in MPR we should have filtered by discharge_date_time for imaging too, which is currently not implemented. For the functions in Rgemini, by default, we would want to exclude records before triage_date_time and after discharge_date_time. To add these filters I think it is better we address the coverage issue first, because we will have coverage issue for er table as well as the clinical table we are looking at.

[guoyi-smh]

@guoyi-smh one more general comment/question (for a future sprint): Some of the filtering by date-time variables happens within the SQL query now, which probably makes it more efficient, but with this approach it’s not obvious how many date-time variables are missing/invalid. I’ve been adding a new utility function that transforms/checks date-time variables (see issue #76), so I think eventually, I’d like to apply this within these 3 functions as well so users are informed about missing/invalid date-times? This might be particularly relevant for imaging, so we can inform users how missing order date-times are handled. That would mean we’d query all entries first and then perform the filtering by date-times after. Any thoughts on this approach? If you agree, I can open a new issue for that so we can tackle that in a future update.

Agreed! I hesitated on this but chose the performance because I thought the numbers were not reported now anyways. As the user I would definitely like to see those numbers instead of just getting the final result table.

[loffleraSMH]

Thanks so much for implementing this Yishan! It’s nice to have this improved consistency across the 3 functions and also have the additional warning messages. I tested all functions to make sure they run as expected. For n_rbc_transfusions, the new output is a tiny bit different from before (when exclude_ed = TRUE) but I believe that’s simply due to the fact that the previous version excluded transfusions after discharge? For n_routine_bloodwork and n_imaging, the results are identical (if exclude_ed = FALSE) as expected. I can do one more quick round of review once you’ve had a chance to address the comments above (no rush!).

I think you are right, the difference in transfusion should be caused by the filter on discharge_date_time. I dropped that for now to be consistent across 3 function. I had a conversation with Surain on this, and he suggested that in MPR we should have filtered by discharge_date_time for imaging too, which is currently not implemented. For the functions in Rgemini, by default, we would want to exclude records before triage_date_time and after discharge_date_time. To add these filters I think it is better we address the coverage issue first, because we will have coverage issue for er table as well as the clinical table we are looking at.

Yes, good point! I agree, we can update the filters at a later point. I think your current implementation makes sense for now.

[loffleraSMH]

@guoyi-smh one more general comment/question (for a future sprint): Some of the filtering by date-time variables happens within the SQL query now, which probably makes it more efficient, but with this approach it’s not obvious how many date-time variables are missing/invalid. I’ve been adding a new utility function that transforms/checks date-time variables (see issue #76), so I think eventually, I’d like to apply this within these 3 functions as well so users are informed about missing/invalid date-times? This might be particularly relevant for imaging, so we can inform users how missing order date-times are handled. That would mean we’d query all entries first and then perform the filtering by date-times after. Any thoughts on this approach? If you agree, I can open a new issue for that so we can tackle that in a future update.

Agreed! I hesitated on this but chose the performance because I thought the numbers were not reported now anyways. As the user I would definitely like to see those numbers instead of just getting the final result table.

Ok sounds good, I'll go ahead and open a new issue for this so we can update it in the future.

[guoyi-smh]

Hi @loffleraSMH , Thank you so much for you thorough and thoughtful review! I have addressed the issues in the latest push. Please review the update when you have time. Thanks!!