nhs-r-community / NHSRplotthedots

An SPC package to support NHSE/I 'Making Data Count' programme
https://nhs-r-community.github.io/NHSRplotthedots/
Other
47 stars 21 forks source link

Allow x_axis_date_format argument to ptd_create_ggplot() to accept a function not just a string #197

Open francisbarton opened 9 months ago

francisbarton commented 9 months ago

Currently ptd_create_ggplot passes x_axis_date_format to the date_labels argument of ggplot2::scale_x_datetime(), which only accepts a format string.

The labels argument of ggplot2::scale_x_datetime() will accept a function or a string (or a vector).

I wonder if we could build in some logic to PTD that would handle a function if supplied to x_axis_date_format, and pass this to labels instead of date_labels.

I haven't done any work on trying to make this work, just asking the question for now.

tomjemmett commented 9 months ago

We should probably just switch from using the date_labels to labels argument if labels accepts both strings and function?

francisbarton commented 9 months ago

I think there's a good reason why Hadley created a separate date_labels argument, and I suspect that reason is that it is configured to handle strftime() codes, whereas the labels argument, IIUC, isn't. https://ggplot2.tidyverse.org/reference/scale_date.html

I guess we could implement a logic where if it's a string we assume it's a date format string and pass it to date_labels, otherwise we assume it's a function or something else and pass it to labels. But that feels a bit messy.

You can still use a function if you need to, by passing the ptd output on into ggplot2::scale_x_datetime() again, at the cost of incurring a warning that you are formatting the axis twice. And that might be a good enough situation, given that we don't know how many people would ever want this.

ThomUK commented 9 months ago

What use-case do you have in mind @francisbarton? What would this change enable?

francisbarton commented 9 months ago

This actually came from a discussion with a colleague of ours at NUH. They wanted the x axis to display something like "2023-Q3"; that is, quarters (with x_axis_breaks = "3 months" I guess). I don't know how to do this with a plain format string.

Using a function like

quarter_label <- function(x) {
  q <- lubridate::quarter(x)
  y <- lubridate::year(x)
  paste0(y, "-Q", q)
}

is what you need.

Currently you can do this by:

data |>
  ptd_spc(...) |>
  ptd_create_ggplot(...) +
  ggplot2::scale_x_datetime(labels = quarter_label)

- and that might be a good enough workaround for now.