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

get_updatedto_text() gives incorrect output #117

Closed francisbarton closed 23 hours ago

francisbarton commented 8 months ago

I've made a mistake in code here that I've contributed. Lacking tests :-(

Basically as an example: if you have monthly data and the most recent data point is 2024-02-01 00:00:00 (a datetime), then the ceiling_date() step here won't bump the date to the next month (this is correct and intended behaviour in ceiling_date()*).

So the "updated to" text will end up saying "Jan 31 2024" rather than the intended "Feb 29 2024".

The fix for this is probably either:

Edit: An even more robust and sensible fix would be to leave the last date data as it is but round it to a presentation format appropriate to the aggregation. So for monthly data to February, instead of saying "February 29 2024" or "February 1 2024" it would say "February 2024". I think the lag is calculated separately so this wouldn't affect that.

* see docs and examples:

> behaviour of change_on_boundary

> As per default behaviour NULL, instants on the boundary remain the

> same but dates are rounded up

francisbarton commented 8 months ago

A related issue is that the align_rebase_dates() function now doesn't work, as it is looking to match the rebase date (a date) against the dates in the measure data (which are now dttms). So they don't match correctly/as expected/at all.

There are two overall ways we could fix these issues:

  1. In the get_updatedto and align_rebase functions themselves, we can convert the inputs (dttms) to dates and then proceed as before
  2. Or we could a) convert the rebase dates to dttms in that function, and b) use the change_on_boundary parameter in ceiling_date() within the get_updatedto function which would bump the "start of period" dttm to the start of the next period (1 Jan 00:00 would go to 1 Feb 00:00)

The PR coming in uses approach 1.