43 charlson erdiag

[loffleraSMH]

Closes issue #43

Allows users to exclude ER diagnoses by setting erdiag to NULL. Clarified this in the documentation and added a warning message when users don't include ER diagnoses.

Also added input check for ipdiag and erdiag inputs using check_input function.

Note: There may be a merge conflict due to other changes to this function related to issue #48.

[vaakesan-SMH]

I think that this might need a refactor after the changes to the comorbidity functions made in the Elixhauser PR (#48).

[loffleraSMH]

I think that this might need a refactor after the changes to the comorbidity functions made in the Elixhauser PR (#48).

Thanks @vaakesan-SMH! I've made some updates based on the changes in the Elixhauser PR. It's ready for review now.

[loffleraSMH]

Looks great. Small nit but feel free to take or leave.

I might also suggest somehow demonstrating this new behaviour in the function tests. Usually in the diagnoses_at_admission tests, we supply an empty erdiag dataframe (the tests are mostly focused on the ipdiag rules). Perhaps in one of the tests, we can change this empty dataframe to a NULL, like this:

test_that("exclude type 6 and include MRDx if not also type 2", {
  ipdiag <- data.table(
    genc_id = rep(11111111, times = 2),
    diagnosis_code = c("K766", "D529"),
    diagnosis_type = c("M", 6)
  )

  ercols <- c("genc_id", "er_diagnosis_code", "er_diagnosis_type")
  erdiag <- NULL ## <----- HERE (used to be data.table(matrix(nrow = 0, ncol = length(ercols)))
  colnames(erdiag) <- ercols

  res <- diagnoses_at_admission(ipdiag, erdiag)

  expect_equal(
    data.table(
      genc_id = c(11111111),
      diagnosis_code = c("K766"),
      diagnosis_type = c("M")
    ),
    res
  )

  ipdiag <- data.table(
    genc_id = rep(11111111, times = 3),
    diagnosis_code = c("K766", "D529", "K766"),
    diagnosis_type = c("M", 6, 2)
  )

  res <- diagnoses_at_admission(ipdiag, erdiag)

  expect_equal(nrow(res), 0)
})

Thanks @vaakesan-SMH for the review and suggestions! I've made the changes you suggested and also updated one of the tests to include a scenario with erdiag = NULL.

[vaakesan-SMH]

Thanks @vaakesan-SMH for the review and suggestions! I've made the changes you suggested and also updated one of the tests to include a scenario with erdiag = NULL.

Looks great, feel free to go ahead with the merge