pharmaverse/datacutr

Peer review - special_dm_cut.R

Closed this issue · 0 comments

reesnj commented

Hello @barnett11 - please find a few comments for you to consider for special_dm_cut.R below.

  • In the description, I think it's worth clarifying that this function also applies the patient cut to the DM domain. At the moment, it's not clear that this patient cut is being performed unless you look into the code.
  • Might be nice if under @return you could specify what the flagging variables will be called in the resulting dataset (i.e; DCUT_TEMP_DTHCHANGE for deaths after datacut date and DCUT_TEMP_REMOVE for patients not in DCUT).
  • Just curious whether you are happy to assume that the death date will always be stored in DTHDTC, rather than letting the user have a choice here? I presume that the reason you have not added this flexibility is because DTHDTC is a SDTM standard so should be used across all companies? If that's the case, happy to leave as is.
  • There are no assertions for the dataset_dm input. I think we should add some assertions here to check that this is a dataframe which includes both USUBJID and DTHDTC.
  • Might be being pedantic here but the example on the site would be nicer to view if you didn't load in the two packages at the start, since this results in lots of irrelevant notes appearing in the example. I wonder if you can just use dplyr::mutate for example rather than loading in dplyr entirely? Although not sure how you could update the pipe (might just need to remove it from the example). No worries if you want to leave the example as is - not a big deal.