makes target argument act like rebase argument

[tomjemmett]

resolves #79

• adds ptd_target() function so user's can set the target for all facet's or individually

• adds ptd_add_target_column(), moving logic out of ptd_spc_standand()

• [x] added unit tests and checked code coverage with covr::report() (should aim for 100%)

• [x] ran devtools::document()

• [x] ran lintr::lint_package() and resolved all lint warnings and notes

• [x] ran styler::style_pkg() to make sure code matches the style guidelines

• [x] ran R-CMD CHECK and resolved all issues

[tomjemmett]

have you any examples of what you were trying to run? as in something like ptd_target(list(a = 1, b = 2)) rather than ptd_target(a = 1, b = 2)? my preference (as evidenced by how I wrote the function 😆 ) would be to just support passing in as dots, it's cleaner than expecting a list... but that does fall down if you are trying to run this programmatically from a config file (though, you could always use purrr::lift_dl(ptd_target)(your_list_here)

[chrisreading01]

Here is an example of what I was playing with - my thinking was that if I was doing faceted SPCs for specialties, I'll probably be fetching the targets by specialty from a SQL table where they are stored, just in case targets ever change. It's then finding the simplest way to pass that target data to the function I guess:

pretendIPulledThisFromSQL <- data.frame(
  org = c("RRK","RWP")
  ,target = c(10000,30000)
)

targetList <- as.list(pretendIPulledThisFromSQL$target)
names(targetList) <- pretendIPulledThisFromSQL$org

NHSRdatasets::ae_attendances %>%
  filter(org_code %in% c('RWP','RRK'), type == 1) %>%
  NHSRplotthedots::ptd_spc(
    value_field = "attendances"
    ,date_field = "period"
    ,facet_field = "org_code"
    ,target = ptd_target(targetList)
    )

[chrisreading01]

In fact I think I'm just being stupid. If you pass the list without the helper function it achieves what I was going for. Maybe just an addendum to the help file to say that if you want to use a pre-defined and named list, supply that independently instead of through the helper function. For the benefit of twits like me!

Edit: or maybe a helpful error message that tells you not to use the function if it sees you have passed a list

[tomjemmett]

added some more details to the documentation 👍

[tomjemmett]

what I added:

Details

This function is a helper to provide data in the correct format for use with ptd_spc(). See Value section for details of return type. If you are trying to do something like ptd_spc(list_of_values) then you can skip using the function and just use list_of_values, so long as the list meets the requirements as listed above.

Value

returns either:

• a single numeric value, in this case all facets in the plot will use this target value
• a named list of single numeric values, where each item is named as for one of the facet's in the plot. If a facet isn't specified then it will not have a target

[tomjemmett]

@chrisreading01 hopefully the additional documentation and examples I have added make it clearer how to use this. For power users this function and ptd_rebase() may not really be required. The benefit of using them is to give documentation for the format of the inputs for these arguments in ptd_spc() and the ability to add validation to provided values. If you are automating it can be ignored 😄

If you are otherwise happy with this, ready to merge? Or if you can think of any ways to improve the documentation feel free to push up some revisions to this branch 👍

[chrisreading01]

Yep, I'm happy for this one to be merged!