ThomUK / SPCreporter

Creates Metric Reports using Statistical Process Control in the NHS style
https://thomuk.github.io/SPCreporter/
Other
6 stars 2 forks source link

Plotthedots generates an error if multiple events are submitted on a single day #97

Open francisbarton opened 9 months ago

francisbarton commented 9 months ago

If more than two rare events occur on a single date, NHSRplotthedots::ptd_spc(), which is called by SPCreporter:::make_spc_data(), gives an error.

(Error is generated specifically by ptd_validate_spc_options)

Reprex:

library(lubridate, warn.conflicts = FALSE)

test_data <- list(
  month = tibble::tribble(
    ~ref, ~measure_name, ~comment, ~`2023-12-01`,
    "100", "More data", "Test", 1
  ),
  events = tibble::tribble(
    ~ref, ~measure_name, ~comment, ~event_date_or_datetime,
    "1", "Test data", "Reprex for duplicate date", as_date("2023-12-18"),
    "1", "Test data", "Reprex for duplicate date", as_date("2023-12-18"),
    "1", "Test data", "Reprex for duplicate date", as_date("2023-12-18")
  )
)

test_rc <- tibble::tribble(
  ~ref, ~measure_name, ~domain, ~spc_chart_type, ~aggregation, ~report_comment,
  "1", "Test data", NA_character_, "t", "none", NA_character_
)

test_mc <- tibble::tribble(
  ~ref, ~measure_name, ~unit, ~data_source, ~data_owner, ~accountable_person, ~improvement_direction, ~target, ~target_set_by, ~data_quality, ~rebase_dates, ~rebase_comment,
  "1", "Test data", "integer", "My brain", "Me", "nobody", "decrease", 0, "Me", "GGGG", "", ""
)

data_cutoff_dttm <- as_datetime("2023-12-31 23:59:59")
test_data_bundle <- SPCreporter::spcr_make_data_bundle(
  test_data, test_rc, test_mc, data_cutoff_dttm
)
#> ℹ check_for_optional_columns: Optional column 'allowable_days_lag' is missing.
#> Adding it.

test_data_bundle |>
  dplyr::select(all_of(c(
    "target",
    "rebase_dates",
    "improvement_direction",
    "measure_data"
  ))) |>
  purrr::pmap(SPCreporter:::make_spc_data, .progress = "SPC data")
#> Error in `purrr::pmap()`:
#> ℹ In index: 1.
#> Caused by error:
#> ! duplicate rows found in 'date'
#> Backtrace:
#>      ▆
#>   1. ├─purrr::pmap(...)
#>   2. │ └─purrr:::pmap_("list", .l, .f, ..., .progress = .progress)
#>   3. │   ├─purrr:::with_indexed_errors(...)
#>   4. │   │ └─base::withCallingHandlers(...)
#>   5. │   ├─purrr:::call_with_cleanup(...)
#>   6. │   └─SPCreporter (local) .f(...)
#>   7. │     ├─NHSRplotthedots::ptd_spc(...) at SPCreporter/R/spcr_make_report.R:205:3
#>   8. │     └─NHSRplotthedots:::ptd_spc.data.frame(...) at NHSRplotthedots/R/ptd_spc.R:97:3
#>   9. │       └─NHSRplotthedots:::ptd_validate_spc_options(options, .data) at NHSRplotthedots/R/ptd_spc.R:135:3
#>  10. │         └─assertthat::assert_that(...) at NHSRplotthedots/R/ptd_validate_spc_options.R:25:3
#>  11. │           └─base::stop(assertError(attr(res, "msg")))
#>  12. └─purrr (local) `<fn>`(`<assrtErr>`)
#>  13.   └─cli::cli_abort(...)
#>  14.     └─rlang::abort(...)
ThomUK commented 9 months ago

Thanks - we need to correct this with the help of some unit tests to document and exercise the edge-cases in the test suite. Hopeful that using date-times not just dates will solve, but I need to investigate.