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

#25 disabilities #51

Closed loffleraSMH closed 8 months ago

loffleraSMH commented 9 months ago

Adds a new function to derive disability flag.

Closes issue #25

To test this function, you will need to re-install the package from the corresponding branch in order to successfully read the data file containing the diagnosis code mapping (e.g., pak::pkg_install("GEMINI-Medicine/Rgemini@25_disabilities").

Note: The documentation still needs to be improved to provide a better reference/rationale for when/why users may want to exclude ER diagnosis codes (also see issues #43 and #44 - I've added a note in both issues that this also needs to be updated in disability as well so this could be tackled when working on those issues).

loffleraSMH commented 8 months ago

Hey @gemini-wenb, I don't know if you already started reviewing this function, but I just pushed some more changes:

  1. I liked your version of the component_wise output in frailty_score much better than my original implementation of it, so I changed it to be more similar to yours (with one small difference: I'm also including the code descriptions in the output, which I thought might be helpful for users to understand/check the mappings).
  2. I made some small changes to the mapping file, so you'll likely have to re-install the branch for the function to work as expected.
  3. I also added the input checks that were previously commented out because the check_input function has now been merged into develop.
loffleraSMH commented 8 months ago

Thanks for developing this function and addressing comments in my previous review. Everything looks good - function works as expected with good documentation and unit tests. I only have 2 minor suggestions on the documentation, but happy to keep as it is. Ready to merge!

Thanks for reviewing @gemini-wenb! I made the 2 small changes to the documentation you suggested and will go ahead and merge now.