epinowcast icon indicating copy to clipboard operation
epinowcast copied to clipboard

Add support for calendar month/year timesteps

Open seabbs opened this issue 2 years ago • 1 comments

At the moment (as of #303) flexible numeric timesteps are supported. However, often data is reported by a calendar period that may contain a variable number of days. For example monthly data. Obviously, here it is preferable to use a daily or weekly model as having varying timesteps is likely to break the assumed mechanism but for many users, this may not be feasible.

For this reason, it makes senses to extend get_internal_timestep(), check_timestep(), check_timestep_by_date(), enw_complete_dates() and enw_preprocess_data() to support calendar months as a priority with years being a potential extension (and likely one that is relatively trivial once months are implemented).

Note that #303 implemented support for checking and completing sequential months but not non-sequential months. If users specify a month then enw_preprocess_data() returns an error. This is because internally timesteps are handled by being scaled to a daily scale which for months does not exist. Potentially it would be enough to approximate this by assuming a month is ~ 28 days etc but its unclear what ramifications this would have. Alternatively a fairly substantial rewrite might be needed to account for monthly data.

This issue links to #81 and #240.

seabbs avatar Aug 11 '23 14:08 seabbs

Summary

This issue documents month timestep support code that was removed from the codebase in PR #625. This reference will be useful if month support is re-implemented in the future (see #309).

Code Removed

1. check_calendar_timestep() function (R/check.R)

  • Purpose: Validated that dates were sequential with month-based timestep
  • Key functionality:
    • Used lubridate::%m-% to calculate month differences
    • Checked for sequential dates at month boundaries
    • Provided errors for non-sequential or missing months

2. Month branch in check_timestep() (R/check.R)

  • Removed: Conditional logic that called check_calendar_timestep() when timestep = "month"
  • Now: All timestep checking goes through check_numeric_timestep() only

3. Month handling in .format_delay_with_units() (R/utils.R)

  • Removed:
    • Early return for month timesteps before conversion to days
    • Month case in switch statement for timestep_unit
    • Month-specific pluralization logic
    • Month-specific formatting branch
  • Rationale: Since get_internal_timestep() throws an error for "month", this code was unreachable

4. Documentation updates

  • Removed check_calendar_timestep.Rd from man/
  • Updated @inheritParams references to document parameters directly
  • Updated function documentation to remove month-specific language

Code Retained

get_internal_timestep() error handling

The error for month timesteps was retained in get_internal_timestep():

month = cli::cli_abort(
  paste0(
    "Calendar months are not currently supported. Consider using an ",
    "approximate number of days (i.e. 28), a different timestep ",
    "(i.e.'week'), or commenting on issue #309. "
  )
)

This provides a clear error message when users attempt to use month timesteps.

Metadata generation

Month metadata column generation in enw_add_metaobs_features() was retained as it's a simple derived column independent of timestep functionality.

Related Issues

  • #309: Original issue tracking month support
  • #623: PR that removed month timestep support
  • #625: PR that cleaned up remaining month-related code

For Future Implementation

If re-implementing month support:

  1. Review the removed check_calendar_timestep() function logic
  2. Consider how to handle variable-length months (28-31 days)
  3. Update get_internal_timestep() to return appropriate value
  4. Restore month branches in .format_delay_with_units()
  5. Add comprehensive tests for month-based timesteps
  6. Update documentation throughout

seabbs avatar Oct 14 '25 16:10 seabbs