Merge branch 'master' into fwrite_forceDecimal

ben-schwen · ben-schwen · commit dd191661fde3 · 2025-08-05T18:02:06.000+02:00
diff --git a/NEWS.md b/NEWS.md
@@ -105,6 +105,8 @@
 
 17. `t1 - t2`, where one is an `IDate` and the other is a `Date`, are now consistent with the case where both are `IDate` or both are `Date`, [#4749](https://github.com/Rdatatable/data.table/issues/4749). Thanks @George9000 for the report and @MichaelChirico for the fix.
 
+18. `fwrite` now allows `dec` to be the same as `sep` for edge cases where only one will be written, e.g. 0-row or 1-column tables. [#7227](https://github.com/Rdatatable/data.table/issues/7227). Thanks @MichaelChirico for the report and @venom1204 for the fix.
+
 ### NOTES
 
 1. The following in-progress deprecations have proceeded:
diff --git a/R/fwrite.R b/R/fwrite.R
@@ -45,7 +45,7 @@ fwrite = function(x, file="", append=FALSE, quote="auto",
     is.character(sep) && length(sep)==1L && (nchar(sep) == 1L || identical(sep, "")),
     is.character(sep2) && length(sep2)==3L && nchar(sep2[2L])==1L,
     is.character(dec) && length(dec)==1L && nchar(dec) == 1L,
-    dec != sep,  # sep2!=dec and sep2!=sep checked at C level when we know if list columns are present
+    `dec and sep must be distinct whenever both might be needed` = (!NROW(x) || NCOL(x) <= 1L || dec != sep),  # sep2!=dec and sep2!=sep checked at C level when we know if list columns are present
     is.character(eol) && length(eol)==1L,
     length(qmethod) == 1L && qmethod %chin% c("double", "escape"),
     length(compress) == 1L && compress %chin% c("auto", "none", "gzip"),
diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw
@@ -10977,9 +10977,9 @@ test(1732.7, fwrite(DT, quote='auto'), output='A,B\n,5\nNA,7\n"",0\nmonty,')
 test(1732.8, fwrite(DT, quote='auto', na="NA"), output='A,B\nNA,5\n"NA",7\n"",0\n"monty",NA')
 
 # dec=","
-test(1733.1, fwrite(data.table(pi),dec=","), error=base_messages$stopifnot("dec != sep"))
+# Test 1733.1 removed, see #7227
 test(1733.2, fwrite(data.table(c(1.2,-8.0,pi,67.99),1:4),dec=",",sep=";"),
-           output="V1;V2\n1,2;1\n-8;2\n3,14159265358979;3\n67,99;4")
+             output="V1;V2\n1,2;1\n-8;2\n3,14159265358979;3\n67,99;4")
 
 # fwrite implied and actual row.names
 DT = data.table(foo=1:3,bar=c(1.2,9.8,-6.0))
@@ -21594,6 +21594,15 @@ test(2336.4, all.equal(as.Date(t2) - t1, t2 - t1))
 test(2336.5, all.equal(t1 - as.Date(t2), t1 - t2))
 test(2336.6, all.equal(t2 - as.Date(t1), t2 - t1))
 
+# fwrite: allow dec=',' with single column, #7227
+test(2337.1, fwrite(data.table(1), dec=","), NULL)
+if (getRversion() >= "4.0.0") { # rely on stopifnot(named = ...) for correct message
+  test(2337.2, fwrite(data.table(0.1, 0.2), dec=",", sep=","), error = "dec and sep must be distinct")
+}
+test(2337.3, is.null(fwrite(data.table(c(0.1, 0.2)), dec=",", sep="\t")))
+test(2337.4, is.null(fwrite(data.table(a=numeric(), b=numeric()), dec=",", sep=",")))
+test(2337.5, is.null(fwrite(data.table(a=numeric()), dec=",", sep=",")))
+
 # 2864 force decimal points for whole numbers in numeric columns
 dd = data.table(x=c(1, 2, 3))
 di = data.table(x=c(1L, 2L, 3L))
@@ -21603,9 +21612,11 @@ test(2337.3, capture.output(fwrite(dd, forceDecimal=FALSE)), c("x", "1", "2", "3
 test(2337.4, capture.output(fwrite(di, forceDecimal=TRUE)), c("x", "1", "2", "3"))
 test(2337.5, capture.output(fwrite(data.table(x=c(0.)), forceDecimal=TRUE)), c("x", "0."))
 test(2337.6, capture.output(fwrite(data.table(x=c(-0.)), forceDecimal=TRUE)), c("x", "0."))
+test(2337.7, capture.output(fwrite(data.table(x=c(0.00)), forceDecimal=TRUE)), c("x", "0."))
 # round trip
 local({
   f <- tempfile(); on.exit(unlink(f))
-  test(2337.7, {fwrite(dd, f, forceDecimal=TRUE); fread(f)}, dd)
-  test(2337.8, {fwrite(dd, f, forceDecimal=FALSE); fread(f)}, di)
+  test(2337.8, {fwrite(dd, f, forceDecimal=TRUE); fread(f)}, dd)
+  test(2337.9, {fwrite(dd, f, forceDecimal=FALSE); fread(f)}, di)
 })
+
diff --git a/src/fmelt.c b/src/fmelt.c
@@ -103,12 +103,14 @@ static const char *concat(SEXP vec, SEXP idx) {
 // with missing inputs, and -1 in the positions with column names not
 // found. Column names not found will eventually cause error via
 // uniq_diff().
-SEXP chmatch_na(SEXP x, SEXP table){
-  SEXP ans;
-  PROTECT(ans = chmatch(x, table, -1));
-  for(int i=0; i<length(ans); i++){
-    if(STRING_ELT(x, i) == NA_STRING){
-      INTEGER(ans)[i] = NA_INTEGER;
+SEXP chmatch_na(SEXP x, SEXP table)
+{
+  SEXP ans = chmatch(x, table, -1);
+  PROTECT(ans);
+  int *restrict target = INTEGER(ans);
+  for (int i = 0; i < length(ans); i++) {
+    if (STRING_ELT(x, i) == NA_STRING) {
+      target[i] = NA_INTEGER;
     }
   }
   UNPROTECT(1);
diff --git a/src/frank.c b/src/frank.c
@@ -3,22 +3,26 @@
 // #include <signal.h> // the debugging machinery + breakpoint aidee
 // raise(SIGINT);
 
-SEXP dt_na(SEXP x, SEXP cols) {
-  int n=0, elem;
-
+SEXP dt_na(SEXP x, SEXP cols)
+{
   if (!isNewList(x)) internal_error(__func__, "Argument '%s' to %s is type '%s' not '%s'", "x", "Cdt_na", type2char(TYPEOF(x)), "list"); // # nocov
   if (!isInteger(cols)) internal_error(__func__, "Argument '%s' to %s is type '%s' not '%s'", "cols", "Cdt_na", type2char(TYPEOF(cols)), "integer"); // # nocov
-  for (int i=0; i<LENGTH(cols); ++i) {
-    elem = INTEGER(cols)[i];
-    if (elem<1 || elem>LENGTH(x))
-      error(_("Item %d of 'cols' is %d which is outside 1-based range [1,ncol(x)=%d]"), i+1, elem, LENGTH(x));
-    if (!n) n = length(VECTOR_ELT(x, elem-1));
+
+  int n = 0;
+  const int numCols = LENGTH(cols);
+  const int* col_ints = INTEGER_RO(cols);
+  for (int i = 0; i < numCols; i++) {
+    const int elem = col_ints[i];
+    if (elem < 1 || elem > LENGTH(x))
+      error(_("Item %d of 'cols' is %d which is outside 1-based range [1,ncol(x)=%d]"), i + 1, elem, LENGTH(x));
+    if (!n) n = length(VECTOR_ELT(x, elem - 1));
   }
   SEXP ans = PROTECT(allocVector(LGLSXP, n));
   int *ians = LOGICAL(ans);
-  for (int i=0; i<n; ++i) ians[i]=0;
-  for (int i=0; i<LENGTH(cols); ++i) {
-    SEXP v = VECTOR_ELT(x, INTEGER(cols)[i]-1);
+  memset(ians, 0, n * sizeof(int));
+
+  for (int i = 0; i < numCols; i++) {
+    SEXP v = VECTOR_ELT(x, col_ints[i]-1);
     if (!length(v) || isList(v)) continue; // like stats:::na.omit.data.frame, skip pairlist columns
     if (n != length(v))
       error(_("Column %d of input list x is length %d, inconsistent with first column of that item which is length %d."), i+1,length(v),n);
diff --git a/src/idatetime.c b/src/idatetime.c
@@ -1,9 +1,9 @@
 #include "data.table.h"
 
-#define YEARS400 146097
-#define YEARS100 36524
-#define YEARS4 1461
-#define YEARS1 365
+static const int YEARS400 = 146097;
+static const int YEARS100 = 36524;
+static const int YEARS4 = 1461;
+static const int YEARS1 = 365;
 
 typedef enum { YDAY, WDAY, MDAY, WEEK, MONTH, QUARTER, YEAR, YEARMON, YEARQTR} datetype;
 
@@ -124,37 +124,40 @@ void convertSingleDate(int x, datetype type, void *out)
 SEXP convertDate(SEXP x, SEXP type)
 {
     if (!isInteger(x)) error(_("x must be an integer vector"));
-    const int *ix = INTEGER(x);
+    const int *ix = INTEGER_RO(x);
     const int n = length(x);
     if (!isString(type) || length(type) != 1)
         internal_error(__func__, "invalid type for, should have been caught before"); // # nocov
-    datetype ctype=0;
+    datetype ctype = 0;
     bool ansint = true;
-    if (!strcmp(CHAR(STRING_ELT(type, 0)), "yday")) ctype = YDAY;
-    else if (!strcmp(CHAR(STRING_ELT(type, 0)), "wday")) ctype = WDAY;
-    else if (!strcmp(CHAR(STRING_ELT(type, 0)), "mday")) ctype = MDAY;
-    else if (!strcmp(CHAR(STRING_ELT(type, 0)), "week")) ctype = WEEK;
-    else if (!strcmp(CHAR(STRING_ELT(type, 0)), "month")) ctype = MONTH;
-    else if (!strcmp(CHAR(STRING_ELT(type, 0)), "quarter")) ctype = QUARTER;
-    else if (!strcmp(CHAR(STRING_ELT(type, 0)), "year")) ctype = YEAR;
-    else if (!strcmp(CHAR(STRING_ELT(type, 0)), "yearmon")) { ctype = YEARMON; ansint = false; }
-    else if (!strcmp(CHAR(STRING_ELT(type, 0)), "yearqtr")) { ctype = YEARQTR; ansint = false; }
-    else internal_error(__func__, "invalid type for, should have been caught before"); // # nocov
-
-    SEXP ans;
+
+    const char* ctype_str = CHAR(STRING_ELT(type, 0));
+    if (!strcmp(ctype_str, "yday")) ctype = YDAY;
+    else if (!strcmp(ctype_str, "wday")) ctype = WDAY;
+    else if (!strcmp(ctype_str, "mday")) ctype = MDAY;
+    else if (!strcmp(ctype_str, "week")) ctype = WEEK;
+    else if (!strcmp(ctype_str, "month")) ctype = MONTH;
+    else if (!strcmp(ctype_str, "quarter")) ctype = QUARTER;
+    else if (!strcmp(ctype_str, "year")) ctype = YEAR;
+    else if (!strcmp(ctype_str, "yearmon")) { ctype = YEARMON; ansint = false; }
+    else if (!strcmp(ctype_str, "yearqtr")) { ctype = YEARQTR; ansint = false; }
+    else internal_error(__func__, "invalid type, should have been caught before"); // # nocov
+    
     if (ansint) {
-        ans = PROTECT(allocVector(INTSXP, n));
+        SEXP ans = PROTECT(allocVector(INTSXP, n));
         int *ansp = INTEGER(ans);
-        for (int i=0; i < n; ++i) {
+        for (int i = 0; i < n; i++) {
             convertSingleDate(ix[i], ctype, &ansp[i]);
         }
+        UNPROTECT(1);
+        return ans;
     } else {
-        ans = PROTECT(allocVector(REALSXP, n));
+        SEXP ans = PROTECT(allocVector(REALSXP, n));
         double *ansp = REAL(ans);
-        for (int i=0; i < n; ++i) {
+        for (int i = 0; i < n; i++) {
             convertSingleDate(ix[i], ctype, &ansp[i]);
         }
+        UNPROTECT(1);
+        return ans;
     }
-    UNPROTECT(1);
-    return ans;
 }
diff --git a/src/nqrecreateindices.c b/src/nqrecreateindices.c
@@ -1,23 +1,24 @@
 #include "data.table.h"
+#include <string.h>
 
 // TODO: Add oxygen style comments and cleanup var names.
 // See other TODOs inside the function.
 SEXP nqRecreateIndices(SEXP xo, SEXP len, SEXP indices, SEXP nArg, SEXP nomatch)
 {
-  R_len_t n = INTEGER(nArg)[0], xn = length(xo);
+  const R_len_t n = INTEGER_RO(nArg)[0], xn = length(xo);
   SEXP ans, newstarts, newlen;
   ans = PROTECT(allocVector(VECSXP, 2));
   SET_VECTOR_ELT(ans, 0, (newstarts = allocVector(INTSXP, n)));
   SET_VECTOR_ELT(ans, 1, (newlen = allocVector(INTSXP, n)));
 
   int *inewlen = INTEGER(newlen);
-  const int *iindices = INTEGER(indices);
-  const int *ilen = INTEGER(len);
-  const int *ixo = INTEGER(xo);
-  const int inomatch = isNull(nomatch) ? 0 : INTEGER(nomatch)[0];
+  const int *iindices = INTEGER_RO(indices);
+  const int *ilen = INTEGER_RO(len);
+  const int *ixo = INTEGER_RO(xo);
+  const int inomatch = isNull(nomatch) ? 0 : INTEGER_RO(nomatch)[0];
   int *inewstarts = INTEGER(newstarts);
 
-  for (int i = 0; i < n; i++) inewlen[i] = 0;
+  memset(inewlen, 0, n * sizeof(int));
 
   // simplifying logic ... also fixes #2275
   for (int i = 0; i < length(indices); i++) {
