search

GEMINI-Medicine / Rgemini

A custom R package that provides a variety of functions to perform data analyses with GEMINI data
https://gemini-medicine.github.io/Rgemini/
Other
3 stars 0 forks source link

#52 add `dummy_ipadmdad` function #53

Closed loffleraSMH closed 9 months ago

loffleraSMH commented 9 months ago

Closes issue #52

Adds new function to simulate dummy "ipadmdad" table (currently only simulates a subset of variables that are relevant for testing).

Example usage:

# Simulate 10,000 encounters from 10 hospitals for fiscal years 2018-2020.
admdad <- dummy_ipadmdad(n = 10000, n_hospitals = 10, time_period = c(2018, 2020))

Note: This function was originally developed to test plotting functions that are currently still in development (see issue #14). There is no need to review the fine-grained details of the simulations right now (e.g., degree of hospital-level variance). Instead, this is mostly just to have a starting point for a function simulating ipadmdad data, which can be further enhanced in the future depending on the scenarios it is used in (e.g., allow more user-control to create certain test scenarios).

Once this function has been reviewed and approved, the dummy_admdad() function created for the purpose of mlaps testing should be removed and replaced with this more general function.

vaakesan-SMH commented 9 months ago

Depending on which is merged first (this or mlaps), I would like to push a commit to this branch or the mlaps branch that replaces the old dummy_admdad function used in the mlaps unit tests with this new dummy_ipadmdad function.

On second thought, after trying to implement this, I think this might be better handled in a different issue. For the mlaps tests we need fine control over exactly what the admission_date_time is.

loffleraSMH commented 9 months ago

Depending on which is merged first (this or mlaps), I would like to push a commit to this branch or the mlaps branch that replaces the old dummy_admdad function used in the mlaps unit tests with this new dummy_ipadmdad function.

On second thought, after trying to implement this, I think this might be better handled in a different issue. For the mlaps tests we need fine control over exactly what the admission_date_time is.

Ah yes, sorry I missed that aspect. Agree this should be moved to a separate issue then. Will still go ahead and address your comments above so we can merge this into develop (might come in handy for other functions).

loffleraSMH commented 9 months ago

Thanks for reviewing @vaakesan-SMH! I've addressed your comments and fixed the bug.

gemini-wenb commented 8 months ago

@loffleraSMH, the dummy_ipadmdad function has been merged to develop. However, I received the following CI/CD check error about the function after merging develop into the frailty branch. Looks like there is an inconsistency in argument name and use for the function. Could you look into this? image

loffleraSMH commented 8 months ago

Thanks @gemini-wenb, good catch! Sorry, we missed that. I just pushed a fix to develop, can you check if it works now?

gemini-wenb commented 8 months ago

Thanks @gemini-wenb, good catch! Sorry, we missed that. I just pushed a fix to develop, can you check if it works now?

It works now! Thanks for fixing it!