updated logic

Author: venom1204

NEWS.md

@@ -38,6 +38,8 @@
 
 1. `data.table(x=1, <expr>)`, where `<expr>` is an expression resulting in a 1-column matrix without column names, will eventually have names `x` and `V2`, not `x` and `V1`, consistent with `data.table(x=1, <expr>)` where `<expr>` results in an atomic vector, for example `data.table(x=1, cbind(1))` and `data.table(x=1, 1)` will both have columns named `x` and `V2`. In this release, the matrix case continues to be named `V1`, but the new behavior can be activated by setting `options(datatable.old.matrix.autoname)` to `FALSE`. See point 5 under Bug Fixes for more context; this change will provide more internal consistency as well as more consistency with `data.frame()`.
 
+2. The `week()` function will be fixed in a future release to correctly calculate weeks sequentially (days 1-7 as week 1), which is a potential breaking change. For now, the current (buggy) behavior remains the default. A one-time deprecation warning will be issued when the old and new behaviors differ; this can be suppressed with `options(datatable.warn.week.deprecation = FALSE)`. Users can opt-in to the correct behavior now with the temporary option `options(datatable.week.new = TRUE)`. See [#2611](https://github.com/Rdatatable/data.table/issues/2611) for details. Thanks @MichaelChirico for the report.
+
 ### NEW FEATURES
 
 1. New `sort_by()` method for data.tables, [#6662](https://github.com/Rdatatable/data.table/issues/6662). It uses `forder()` to improve upon the data.frame method and also matches `DT[order(...)]` behavior with respect to locale. Thanks @rikivillalba for the suggestion and PR.
@@ -333,8 +335,6 @@
 
 19. Ellipsis elements like `..1` are correctly excluded when searching for variables in "up-a-level" syntax inside `[`, [#5460](https://github.com/Rdatatable/data.table/issues/5460). Thanks @ggrothendieck for the report and @MichaelChirico for the fix.
 
-20. BREAKING CHANGE: `week()` now calculates the week of the year sequentially (days 1-7 are week 1), fixing a bug where the first week could have 6 days. A one-time warning is now issued if this change affects the output for a given input, which can be disabled via `options(datatable.warn.week.change = FALSE)`. [#2611](https://github.com/Rdatatable/data.table/issues/2611). Thanks to @MichaelChirico for the report and @venom1204 for the fix.
-
 ### NOTES
 
 1. The following in-progress deprecations have proceeded:

inst/tests/tests.Rraw

@@ -18384,7 +18384,7 @@ x = c("1111-11-11", "2019-01-01", "2019-02-28", "2019-03-01", "2019-12-31", "202
 test(2236.1, yday(x), c(315L, 1L, 59L, 60L, 365L, 60L, 61L, 366L, 1L, 366L, 60L, NA))
 test(2236.2, mday(x), c(11L, 1L, 28L, 1L, 31L, 29L, 1L, 31L, 1L, 31L, 1L, NA))
 test(2236.3, wday(x), c(7L, 3L, 5L, 6L, 3L, 7L, 1L, 5L, 1L, 2L, 2L, NA))
-test(2236.4, suppressWarnings(week(x)), c(45L, 1L, 9L, 9L, 53L, 9L, 9L, 53L, 1L, 53L, 9L, NA))
+test(2236.4, week(x), c(46L, 1L, 9L, 9L, 53L, 9L, 9L, 53L, 1L, 53L, 9L, NA), warning="The current behavior of data.table::week() is deprecated")
 test(2236.5, month(x), c(11L, 1L, 2L, 3L, 12L, 2L, 3L, 12L, 1L, 12L, 3L, NA))
 test(2236.6, quarter(x), c(4L, 1L, 1L, 1L, 4L, 1L, 1L, 4L, 1L, 4L, 1L, NA))
 test(2236.7, year(x), c(1111L, 2019L, 2019L, 2019L, 2019L, 2020L, 2020L, 2020L, 2040L, 2040L, 2100L, NA))
@@ -21817,6 +21817,7 @@ b
 ', comment.char = '#', strip.white = FALSE, sep = ","), data.table(a=c("  ", "b")))
 
 # week() sequential numbering fix tests #2611
+options(datatable.week.new = TRUE)
 test(2342.1, week(as.IDate("1970-01-01") + 0:7), c(1L,1L,1L,1L,1L,1L,1L,2L))  # Jan 1–7 all week 1, Jan 8 week 2
 test(2342.2, week(as.IDate(c("2012-02-28","2012-02-29","2012-03-01"))), c(9L,9L,9L))  # leap day stays in same week
 test(2342.3, week(as.IDate(c("2019-12-31","2020-01-01"))), c(53L,1L))  # year boundary non-leap → reset to 1

src/idatetime.c

@@ -1,6 +1,6 @@
 #include "data.table.h"
 
-static int week_warning_issued = 0;
+static int week_deprecation_warning_issued = 0;
 
 static const int YEARS400 = 146097;
 static const int YEARS100 = 36524;
@@ -65,8 +65,15 @@ void convertSingleDate(int x, datetype type, void *out)
         if (yday >= YEARS1 + leap)
             yday -= YEARS1 + leap;
         *(int *)out = ++yday;
-        if (type == WEEK)
-            *(int *)out = ((*(int *)out - 1) / 7) + 1;
+        if (type == WEEK) {
+            SEXP opt_new = GetOption(install("datatable.week.new"), R_NilValue);
+            bool use_new_behavior = isLogical(opt_new) && LOGICAL(opt_new)[0] == TRUE;
+            if (use_new_behavior) {
+                *(int *)out = ((*(int *)out - 1) / 7) + 1;
+            } else {
+                *(int *)out = (*(int *)out / 7) + 1;
+            }
+        }
         return;
     }
 
@@ -146,26 +153,43 @@ SEXP convertDate(SEXP x, SEXP type)
     else internal_error(__func__, "invalid type, should have been caught before"); // # nocov
 
     if (ctype == WEEK) {
-        if (!week_warning_issued) {
-            SEXP dt_week_warn_opt = GetOption(install("datatable.warn.week.change"), R_NilValue);
-            if (!isLogical(dt_week_warn_opt) || LOGICAL(dt_week_warn_opt)[0] != FALSE) {
-                for (int i = 0; i < n; i++) {
-                    if (ix[i] == NA_INTEGER) continue;
+        SEXP ans = PROTECT(allocVector(INTSXP, n));
+        int *ansp = INTEGER(ans);
 
-                    int yday;
-                    convertSingleDate(ix[i], YDAY, &yday);
+        SEXP opt_new = GetOption(install("datatable.week.new"), R_NilValue);
+        bool use_new_behavior = isLogical(opt_new) && LOGICAL(opt_new)[0] == TRUE;
 
-                    int old_week = (yday / 7) + 1;
-                    int new_week = ((yday - 1) / 7) + 1;
+        bool can_warn = false;
+        if (!use_new_behavior && !week_deprecation_warning_issued) {
+            SEXP opt_warn_depr = GetOption(install("datatable.warn.week.deprecation"), R_NilValue);
+            can_warn = !isLogical(opt_warn_depr) || LOGICAL(opt_warn_depr)[0] != FALSE;
+        }
 
+        for (int i = 0; i < n; i++) {
+            if (ix[i] == NA_INTEGER) {
+                ansp[i] = NA_INTEGER;
+                continue;
+            }
+            int yday;
+            convertSingleDate(ix[i], YDAY, &yday);
+
+            if (use_new_behavior) {
+                ansp[i] = ((yday - 1) / 7) + 1;
+            } else {
+                int old_week = (yday / 7) + 1;
+                ansp[i] = old_week;
+                if (can_warn) {
+                    int new_week = ((yday - 1) / 7) + 1;
                     if (new_week != old_week) {
-                        Rf_warning("data.table::week() behavior has changed and is now sequential (day 1-7 is week 1). The first week of the year may differ from previous versions. To suppress this warning, run: options(datatable.warn.week.change = FALSE)");
-                        week_warning_issued = 1;
-                        break;
+                        Rf_warning("The current behavior of data.table::week() is deprecated and will be fixed in a future version to be sequential (day 1-7 is week 1). To opt-in to the new behavior now, run: options(datatable.week.new = TRUE). To suppress this warning, run: options(datatable.warn.week.deprecation = FALSE)");
+                        week_deprecation_warning_issued = 1;
+                        can_warn = false;
                     }
                 }
             }
         }
+        UNPROTECT(1);
+        return ans;
     }
 
     if (ansint) {