From 8280e2d450dac2af0e52404d86f441aae8223b5e Mon Sep 17 00:00:00 2001 From: Mukul Kumar Date: Thu, 21 Aug 2025 20:16:05 +0530 Subject: [PATCH 01/11] use integer indices for measure.vars and error on non scalar aggregates --- R/fcast.R | 6 +-- inst/tests/tests.Rraw | 10 ++-- src/fmelt.c | 106 +++++++++++++++++++++++++----------------- 3 files changed, 70 insertions(+), 52 deletions(-) diff --git a/R/fcast.R b/R/fcast.R index 2e0aee57da..f059812dd0 100644 --- a/R/fcast.R +++ b/R/fcast.R @@ -186,11 +186,7 @@ dcast.data.table = function(data, formula, fun.aggregate = NULL, sep = "_", ..., maybe_err = function(list.of.columns) { if (!all(lengths(list.of.columns) == 1L)) { msg = gettext("Aggregating functions should take a vector as input and return a single value (length=1), but they do not, so the result is undefined. Please fix by modifying your function so that a single value is always returned.") - if (is.null(fill)) { # TODO change to always stopf #6329 - stop(msg, domain=NA, call. = FALSE) - } else { - warning(msg, domain=NA, call. = FALSE) - } + stop(msg, domain=NA, call. = FALSE) } list.of.columns } diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index d7ffc476bb..d0d3b9e681 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -17417,13 +17417,13 @@ exid = data.table(id=1, expected) test(2182.3, melt(DTid, measure.vars=list(a=c(NA,1), b=2:3), id.vars="id"), exid) test(2182.4, melt(DTid, measure.vars=list(a=c(NA,"a2"), b=c("b1","b2")), id.vars="id"), exid) test(2182.5, melt(DT.wide, measure.vars=list(a=c(NA,1), b=2:3), na.rm=TRUE), data.table(variable=factor(2), a=2, b=2)) -test(2182.6, melt(DT.wide, measure.vars=list(b=c("b1","b2"))), data.table(a2=2, variable=factor(c("b1","b2")), b=c(1,2)), warning="measure.vars is a list with length=1") # measure.vars named list length=1, #5065 -# consistency between measure.vars=list with length=1 and length>1, #5209 -test(2182.71, melt(DT.wide, measure.vars=list("a2"), variable.factor=TRUE), data.table(b1=1, b2=2, variable=factor("a2"), value=2), warning="measure.vars is a list with length=1") +test(2182.6, melt(DT.wide, measure.vars=list(b=c("b1","b2"))), data.table(a2=2, variable=factor(1:2), b=c(1,2))) # list yields indices +# consistency between measure.vars=list with length=1 and length>1 now uses indices for list case, #5209 +test(2182.71, melt(DT.wide, measure.vars=list("a2"), variable.factor=TRUE), data.table(b1=1, b2=2, variable=factor(1L), value=2)) test(2182.72, melt(DT.wide, measure.vars=c("a2"), variable.factor=TRUE), data.table(b1=1, b2=2, variable=factor("a2"), value=2)) -test(2182.73, melt(DT.wide, measure.vars=list("a2"), variable.factor=FALSE), data.table(b1=1, b2=2, variable="a2", value=2), warning="measure.vars is a list with length=1") +test(2182.73, melt(DT.wide, measure.vars=list("a2"), variable.factor=FALSE), data.table(b1=1, b2=2, variable=1L, value=2)) test(2182.74, melt(DT.wide, measure.vars=c("a2"), variable.factor=FALSE), data.table(b1=1, b2=2, variable="a2", value=2)) -test(2182.75, melt(data.table(a=10, b=20), measure.vars=list(n="a"), variable.factor=FALSE), data.table(b=20, variable="a", n=10), warning="measure.vars is a list with length=1")#thanks @mnazarov +test(2182.75, melt(data.table(a=10, b=20), measure.vars=list(n="a"), variable.factor=FALSE), data.table(b=20, variable=1L, n=10))#thanks @mnazarov ### First block testing measurev # new variable_table attribute for measure.vars, PR#4731 for multiple issues diff --git a/src/fmelt.c b/src/fmelt.c index 54ad7b4de9..7ef7f6b566 100644 --- a/src/fmelt.c +++ b/src/fmelt.c @@ -598,62 +598,84 @@ SEXP getvarcols(SEXP DT, SEXP dtnames, Rboolean varfactor, Rboolean verbose, str if (data->lvalues==1 && length(VECTOR_ELT(data->valuecols, 0)) != data->lmax) internal_error(__func__, "getvarcols %d %d", length(VECTOR_ELT(data->valuecols, 0)), data->lmax); // # nocov if (isNull(data->variable_table)) { - if ((data->lvalues == 1) & data->measure_is_list) { - warning(_("measure.vars is a list with length=1, which as long documented should return integer indices in the 'variable' column, but currently returns character column names. To increase consistency in the next release, we plan to change 'variable' to integer, so users who were relying on this behavior should change measure.vars=list('col_name') (output variable is column name now, but will become column index/integer) to measure.vars='col_name' (variable is column name before and after the planned change).")); - } if (!varfactor) { - SET_VECTOR_ELT(ansvars, 0, target=allocVector(STRSXP, data->totlen)); - if (data->lvalues == 1) {//one value column to output. TODO #5247 change to !data->measure_is_list - const int *thisvaluecols = INTEGER(VECTOR_ELT(data->valuecols, 0)); + if (data->measure_is_list) { + // Return integer indices for list measure.vars (consistency with docs) + SET_VECTOR_ELT(ansvars, 0, target=allocVector(INTSXP, data->totlen)); + int *td = INTEGER(target); for (int j=0, ansloc=0; jlmax; ++j) { const int thislen = data->narm ? length(VECTOR_ELT(data->not_NA_indices, j)) : data->nrow; - SEXP str = STRING_ELT(dtnames, thisvaluecols[j]-1); - for (int k=0; klmax; ++j) { - const int thislen = data->narm ? length(VECTOR_ELT(data->not_NA_indices, j)) : data->nrow; - char buff[20]; - snprintf(buff, sizeof(buff), "%d", level++); // # notranslate - for (int k=0; ktotlen)); + if (data->lvalues == 1) { + const int *thisvaluecols = INTEGER(VECTOR_ELT(data->valuecols, 0)); + for (int j=0, ansloc=0; jlmax; ++j) { + const int thislen = data->narm ? length(VECTOR_ELT(data->not_NA_indices, j)) : data->nrow; + SEXP str = STRING_ELT(dtnames, thisvaluecols[j]-1); + for (int k=0; klmax; ++j) { + const int thislen = data->narm ? length(VECTOR_ELT(data->not_NA_indices, j)) : data->nrow; + char buff[20]; + snprintf(buff, sizeof(buff), "%d", level++); // # notranslate + for (int k=0; ktotlen)); SEXP levels; int *td = INTEGER(target); - if (data->lvalues == 1) {//one value column to output. TODO #5247 change to !data->measure_is_list - SEXP thisvaluecols = VECTOR_ELT(data->valuecols, 0); - int len = length(thisvaluecols); - levels = PROTECT(allocVector(STRSXP, len)); protecti++; - const int *vd = INTEGER(thisvaluecols); - for (int j=0; jnarm && length(VECTOR_ELT(data->not_NA_indices, j))==0)) { numRemove++; md[j]=0; } - } - if (numRemove) { - SEXP newlevels = PROTECT(allocVector(STRSXP, len-numRemove)); protecti++; - for (int i=0, loc=0; imeasure_is_list) { + int nlevel = data->lmax; + levels = PROTECT(allocVector(STRSXP, nlevel)); protecti++; + for (int j=0; jlmax; ++j) { const int thislen = data->narm ? length(VECTOR_ELT(data->not_NA_indices, j)) : data->nrow; - for (int k=0; klmax)); protecti++; - for (int j=0, ansloc=0; jlmax; ++j) { - const int thislen = data->narm ? length(VECTOR_ELT(data->not_NA_indices, j)) : data->nrow; - char buff[20]; - snprintf(buff, sizeof(buff), "%d", nlevel + 1); // # notranslate - SET_STRING_ELT(levels, nlevel++, mkChar(buff)); // generate levels = 1:nlevels - for (int k=0; klvalues == 1) { + SEXP thisvaluecols = VECTOR_ELT(data->valuecols, 0); + int len = length(thisvaluecols); + levels = PROTECT(allocVector(STRSXP, len)); protecti++; + const int *vd = INTEGER(thisvaluecols); + for (int j=0; jnarm && length(VECTOR_ELT(data->not_NA_indices, j))==0)) { numRemove++; md[j]=0; } + } + if (numRemove) { + SEXP newlevels = PROTECT(allocVector(STRSXP, len-numRemove)); protecti++; + for (int i=0, loc=0; ilmax; ++j) { + const int thislen = data->narm ? length(VECTOR_ELT(data->not_NA_indices, j)) : data->nrow; + for (int k=0; klmax)); protecti++; + for (int j=0, ansloc=0; jlmax; ++j) { + const int thislen = data->narm ? length(VECTOR_ELT(data->not_NA_indices, j)) : data->nrow; + char buff[20]; + snprintf(buff, sizeof(buff), "%d", nlevel + 1); // # notranslate + SET_STRING_ELT(levels, nlevel++, mkChar(buff)); // generate levels = 1:nlevels + for (int k=0; k Date: Thu, 21 Aug 2025 23:33:42 +0530 Subject: [PATCH 02/11] 1037 - 404 and 407 --- R/fcast.R | 5 +++++ inst/tests/tests.Rraw | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/R/fcast.R b/R/fcast.R index f059812dd0..b33fcd8dfe 100644 --- a/R/fcast.R +++ b/R/fcast.R @@ -249,3 +249,8 @@ dcast.data.table = function(data, formula, fun.aggregate = NULL, sep = "_", ..., setattr(ans, 'sorted', lhsnames) ans } + +# Update to ensure integer indices are returned for list measure.vars in melt function +# This change is to maintain consistency with the documentation and avoid future warnings. +# The variable column will now return integer indices when measure.vars is a list. +# Users should change measure.vars=list("col_name") to measure.vars="col_name". diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index d0d3b9e681..f3ec668ed2 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -3359,13 +3359,13 @@ Sep,33.5,19.4,15.7,11.9,0,100.8,100.8,0,12.7,12.7,0,174.1") test(1037.401, dim(ans<-melt(x, measure.vars=patterns("^y", "^z"))), INT(4,5)) test(1037.402, ans$variable, factor(c("1","1","2","2"))) test(1037.403, dim(ans<-melt(x, measure.vars=patterns("^y", "^z"), variable.factor=FALSE)), INT(4,5)) - test(1037.404, ans$variable, c("1","1","2","2")) + test(1037.404, ans$variable, INT(1,1,2,2)) x[, c("y1","z1"):=NA] test(1037.405, dim(melt(x, measure.vars=patterns("^y", "^z"))), INT(4,5)) test(1037.406, dim(ans<-melt(x, measure.vars=patterns("^y", "^z"), na.rm=TRUE)), INT(2,5)) test(1037.407, ans$variable, factor(c("2","2"), c("1", "2"))) test(1037.408, dim(ans<-melt(x, measure.vars=patterns("^y", "^z"), na.rm=TRUE, variable.factor=FALSE)), INT(2,5)) - test(1037.409, ans$variable, c("2","2")) + test(1037.409, ans$variable, INT(2,2)) test(1037.410, melt(data.table(NULL), verbose=TRUE), data.table(NULL), output="ncol(data) is 0. Nothing to melt") From 5abd761b3e7526a74046fa1d8c46a9b167cf21fe Mon Sep 17 00:00:00 2001 From: Mukul <145585624+Mukulyadav2004@users.noreply.github.com> Date: Fri, 22 Aug 2025 00:47:47 +0530 Subject: [PATCH 03/11] remove nws item --- R/fcast.R | 5 ----- 1 file changed, 5 deletions(-) diff --git a/R/fcast.R b/R/fcast.R index b33fcd8dfe..f059812dd0 100644 --- a/R/fcast.R +++ b/R/fcast.R @@ -249,8 +249,3 @@ dcast.data.table = function(data, formula, fun.aggregate = NULL, sep = "_", ..., setattr(ans, 'sorted', lhsnames) ans } - -# Update to ensure integer indices are returned for list measure.vars in melt function -# This change is to maintain consistency with the documentation and avoid future warnings. -# The variable column will now return integer indices when measure.vars is a list. -# Users should change measure.vars=list("col_name") to measure.vars="col_name". From e3b8a64bb0c2d8da3648e32e0dca3babbe1ef176 Mon Sep 17 00:00:00 2001 From: Mukul Kumar Date: Fri, 22 Aug 2025 22:53:07 +0530 Subject: [PATCH 04/11] removed extra multiple value column loop --- src/fmelt.c | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/src/fmelt.c b/src/fmelt.c index 7ef7f6b566..32631b1f94 100644 --- a/src/fmelt.c +++ b/src/fmelt.c @@ -610,20 +610,11 @@ SEXP getvarcols(SEXP DT, SEXP dtnames, Rboolean varfactor, Rboolean verbose, str } else { // same behavior for vector measure.vars: variable is column names SET_VECTOR_ELT(ansvars, 0, target=allocVector(STRSXP, data->totlen)); - if (data->lvalues == 1) { - const int *thisvaluecols = INTEGER(VECTOR_ELT(data->valuecols, 0)); - for (int j=0, ansloc=0; jlmax; ++j) { - const int thislen = data->narm ? length(VECTOR_ELT(data->not_NA_indices, j)) : data->nrow; - SEXP str = STRING_ELT(dtnames, thisvaluecols[j]-1); - for (int k=0; klmax; ++j) { - const int thislen = data->narm ? length(VECTOR_ELT(data->not_NA_indices, j)) : data->nrow; - char buff[20]; - snprintf(buff, sizeof(buff), "%d", level++); // # notranslate - for (int k=0; kvaluecols, 0)); + for (int j=0, ansloc=0; jlmax; ++j) { + const int thislen = data->narm ? length(VECTOR_ELT(data->not_NA_indices, j)) : data->nrow; + SEXP str = STRING_ELT(dtnames, thisvaluecols[j]-1); + for (int k=0; k Date: Sat, 23 Aug 2025 04:34:22 +0530 Subject: [PATCH 05/11] revert fcast modifications --- R/fcast.R | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/R/fcast.R b/R/fcast.R index f059812dd0..2e0aee57da 100644 --- a/R/fcast.R +++ b/R/fcast.R @@ -186,7 +186,11 @@ dcast.data.table = function(data, formula, fun.aggregate = NULL, sep = "_", ..., maybe_err = function(list.of.columns) { if (!all(lengths(list.of.columns) == 1L)) { msg = gettext("Aggregating functions should take a vector as input and return a single value (length=1), but they do not, so the result is undefined. Please fix by modifying your function so that a single value is always returned.") - stop(msg, domain=NA, call. = FALSE) + if (is.null(fill)) { # TODO change to always stopf #6329 + stop(msg, domain=NA, call. = FALSE) + } else { + warning(msg, domain=NA, call. = FALSE) + } } list.of.columns } From 879198c138fd6022ea81d18f30d588b4c8e7f257 Mon Sep 17 00:00:00 2001 From: Mukul <145585624+Mukulyadav2004@users.noreply.github.com> Date: Sat, 23 Aug 2025 21:24:39 +0530 Subject: [PATCH 06/11] keep non list path --- src/fmelt.c | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/src/fmelt.c b/src/fmelt.c index 32631b1f94..7fe37ee90d 100644 --- a/src/fmelt.c +++ b/src/fmelt.c @@ -600,7 +600,6 @@ SEXP getvarcols(SEXP DT, SEXP dtnames, Rboolean varfactor, Rboolean verbose, str if (isNull(data->variable_table)) { if (!varfactor) { if (data->measure_is_list) { - // Return integer indices for list measure.vars (consistency with docs) SET_VECTOR_ELT(ansvars, 0, target=allocVector(INTSXP, data->totlen)); int *td = INTEGER(target); for (int j=0, ansloc=0; jlmax; ++j) { @@ -608,13 +607,22 @@ SEXP getvarcols(SEXP DT, SEXP dtnames, Rboolean varfactor, Rboolean verbose, str for (int k=0; ktotlen)); - const int *thisvaluecols = INTEGER(VECTOR_ELT(data->valuecols, 0)); - for (int j=0, ansloc=0; jlmax; ++j) { - const int thislen = data->narm ? length(VECTOR_ELT(data->not_NA_indices, j)) : data->nrow; - SEXP str = STRING_ELT(dtnames, thisvaluecols[j]-1); - for (int k=0; klvalues == 1) {//one value column to output. + const int *thisvaluecols = INTEGER(VECTOR_ELT(data->valuecols, 0)); + for (int j=0, ansloc=0; jlmax; ++j) { + const int thislen = data->narm ? length(VECTOR_ELT(data->not_NA_indices, j)) : data->nrow; + SEXP str = STRING_ELT(dtnames, thisvaluecols[j]-1); + for (int k=0; klmax; ++j) { + const int thislen = data->narm ? length(VECTOR_ELT(data->not_NA_indices, j)) : data->nrow; + char buff[20]; + snprintf(buff, sizeof(buff), "%d", level++); // # notranslate + SEXP s = mkChar(buff); + for (int k=0; kmeasure_is_list) { - int nlevel = data->lmax; - levels = PROTECT(allocVector(STRSXP, nlevel)); protecti++; - for (int j=0; jlmax)); protecti++; + for (int j=0; jlmax; ++j) { char buff[20]; snprintf(buff, sizeof(buff), "%d", j+1); // # notranslate SET_STRING_ELT(levels, j, mkChar(buff)); @@ -633,8 +640,8 @@ SEXP getvarcols(SEXP DT, SEXP dtnames, Rboolean varfactor, Rboolean verbose, str const int thislen = data->narm ? length(VECTOR_ELT(data->not_NA_indices, j)) : data->nrow; for (int k=0; klvalues == 1) { + } else { + if (data->lvalues == 1) { // one value column to output SEXP thisvaluecols = VECTOR_ELT(data->valuecols, 0); int len = length(thisvaluecols); levels = PROTECT(allocVector(STRSXP, len)); protecti++; @@ -644,7 +651,7 @@ SEXP getvarcols(SEXP DT, SEXP dtnames, Rboolean varfactor, Rboolean verbose, str int numRemove = 0; // remove dups and any for which narm and all-NA int *md = INTEGER(m); for (int j=0; jnarm && length(VECTOR_ELT(data->not_NA_indices, j))==0)) { numRemove++; md[j]=0; } + if (md[j]!=j+1 || (data->narm && length(VECTOR_ELT(data->not_NA_indices, j))==0)) { numRemove++; md[j]=0; } } if (numRemove) { SEXP newlevels = PROTECT(allocVector(STRSXP, len-numRemove)); protecti++; @@ -657,7 +664,7 @@ SEXP getvarcols(SEXP DT, SEXP dtnames, Rboolean varfactor, Rboolean verbose, str const int thislen = data->narm ? length(VECTOR_ELT(data->not_NA_indices, j)) : data->nrow; for (int k=0; klmax)); protecti++; for (int j=0, ansloc=0; jlmax; ++j) { From ed1712eac1cc9c257fd46081882d6cc5a062d14e Mon Sep 17 00:00:00 2001 From: Mukul <145585624+Mukulyadav2004@users.noreply.github.com> Date: Mon, 25 Aug 2025 13:04:06 +0530 Subject: [PATCH 07/11] revert --- src/fmelt.c | 35 ++++++++++++++--------------------- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/src/fmelt.c b/src/fmelt.c index 7fe37ee90d..32631b1f94 100644 --- a/src/fmelt.c +++ b/src/fmelt.c @@ -600,6 +600,7 @@ SEXP getvarcols(SEXP DT, SEXP dtnames, Rboolean varfactor, Rboolean verbose, str if (isNull(data->variable_table)) { if (!varfactor) { if (data->measure_is_list) { + // Return integer indices for list measure.vars (consistency with docs) SET_VECTOR_ELT(ansvars, 0, target=allocVector(INTSXP, data->totlen)); int *td = INTEGER(target); for (int j=0, ansloc=0; jlmax; ++j) { @@ -607,22 +608,13 @@ SEXP getvarcols(SEXP DT, SEXP dtnames, Rboolean varfactor, Rboolean verbose, str for (int k=0; ktotlen)); - if (data->lvalues == 1) {//one value column to output. - const int *thisvaluecols = INTEGER(VECTOR_ELT(data->valuecols, 0)); - for (int j=0, ansloc=0; jlmax; ++j) { - const int thislen = data->narm ? length(VECTOR_ELT(data->not_NA_indices, j)) : data->nrow; - SEXP str = STRING_ELT(dtnames, thisvaluecols[j]-1); - for (int k=0; klmax; ++j) { - const int thislen = data->narm ? length(VECTOR_ELT(data->not_NA_indices, j)) : data->nrow; - char buff[20]; - snprintf(buff, sizeof(buff), "%d", level++); // # notranslate - SEXP s = mkChar(buff); - for (int k=0; kvaluecols, 0)); + for (int j=0, ansloc=0; jlmax; ++j) { + const int thislen = data->narm ? length(VECTOR_ELT(data->not_NA_indices, j)) : data->nrow; + SEXP str = STRING_ELT(dtnames, thisvaluecols[j]-1); + for (int k=0; kmeasure_is_list) { - levels = PROTECT(allocVector(STRSXP, data->lmax)); protecti++; - for (int j=0; jlmax; ++j) { + int nlevel = data->lmax; + levels = PROTECT(allocVector(STRSXP, nlevel)); protecti++; + for (int j=0; jnarm ? length(VECTOR_ELT(data->not_NA_indices, j)) : data->nrow; for (int k=0; klvalues == 1) { // one value column to output + } else { // non-list measure.vars keeps legacy name-based levels + if (data->lvalues == 1) { SEXP thisvaluecols = VECTOR_ELT(data->valuecols, 0); int len = length(thisvaluecols); levels = PROTECT(allocVector(STRSXP, len)); protecti++; @@ -651,7 +644,7 @@ SEXP getvarcols(SEXP DT, SEXP dtnames, Rboolean varfactor, Rboolean verbose, str int numRemove = 0; // remove dups and any for which narm and all-NA int *md = INTEGER(m); for (int j=0; jnarm && length(VECTOR_ELT(data->not_NA_indices, j))==0)) { numRemove++; md[j]=0; } + if (md[j]!=j+1 /*dup*/ || (data->narm && length(VECTOR_ELT(data->not_NA_indices, j))==0)) { numRemove++; md[j]=0; } } if (numRemove) { SEXP newlevels = PROTECT(allocVector(STRSXP, len-numRemove)); protecti++; @@ -664,7 +657,7 @@ SEXP getvarcols(SEXP DT, SEXP dtnames, Rboolean varfactor, Rboolean verbose, str const int thislen = data->narm ? length(VECTOR_ELT(data->not_NA_indices, j)) : data->nrow; for (int k=0; klmax)); protecti++; for (int j=0, ansloc=0; jlmax; ++j) { From c0e03ed49d4c43a65ec1915d59c5708030b2bab3 Mon Sep 17 00:00:00 2001 From: Mukul <145585624+Mukulyadav2004@users.noreply.github.com> Date: Tue, 26 Aug 2025 00:48:27 +0530 Subject: [PATCH 08/11] delete multiple output column else branch --- src/fmelt.c | 54 +++++++++++++++++++++-------------------------------- 1 file changed, 21 insertions(+), 33 deletions(-) diff --git a/src/fmelt.c b/src/fmelt.c index 32631b1f94..d00ad4a297 100644 --- a/src/fmelt.c +++ b/src/fmelt.c @@ -634,39 +634,27 @@ SEXP getvarcols(SEXP DT, SEXP dtnames, Rboolean varfactor, Rboolean verbose, str for (int k=0; klvalues == 1) { - SEXP thisvaluecols = VECTOR_ELT(data->valuecols, 0); - int len = length(thisvaluecols); - levels = PROTECT(allocVector(STRSXP, len)); protecti++; - const int *vd = INTEGER(thisvaluecols); - for (int j=0; jnarm && length(VECTOR_ELT(data->not_NA_indices, j))==0)) { numRemove++; md[j]=0; } - } - if (numRemove) { - SEXP newlevels = PROTECT(allocVector(STRSXP, len-numRemove)); protecti++; - for (int i=0, loc=0; ilmax; ++j) { - const int thislen = data->narm ? length(VECTOR_ELT(data->not_NA_indices, j)) : data->nrow; - for (int k=0; klmax)); protecti++; - for (int j=0, ansloc=0; jlmax; ++j) { - const int thislen = data->narm ? length(VECTOR_ELT(data->not_NA_indices, j)) : data->nrow; - char buff[20]; - snprintf(buff, sizeof(buff), "%d", nlevel + 1); // # notranslate - SET_STRING_ELT(levels, nlevel++, mkChar(buff)); // generate levels = 1:nlevels - for (int k=0; kvaluecols, 0); + int len = length(thisvaluecols); + levels = PROTECT(allocVector(STRSXP, len)); protecti++; + const int *vd = INTEGER(thisvaluecols); + for (int j=0; jnarm && length(VECTOR_ELT(data->not_NA_indices, j))==0)) { numRemove++; md[j]=0; } + } + if (numRemove) { + SEXP newlevels = PROTECT(allocVector(STRSXP, len-numRemove)); protecti++; + for (int i=0, loc=0; ilmax; ++j) { + const int thislen = data->narm ? length(VECTOR_ELT(data->not_NA_indices, j)) : data->nrow; + for (int k=0; k Date: Tue, 26 Aug 2025 02:28:44 +0530 Subject: [PATCH 09/11] Fix test cases --- inst/tests/tests.Rraw | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index f3ec668ed2..7b08ad63ee 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -17417,13 +17417,13 @@ exid = data.table(id=1, expected) test(2182.3, melt(DTid, measure.vars=list(a=c(NA,1), b=2:3), id.vars="id"), exid) test(2182.4, melt(DTid, measure.vars=list(a=c(NA,"a2"), b=c("b1","b2")), id.vars="id"), exid) test(2182.5, melt(DT.wide, measure.vars=list(a=c(NA,1), b=2:3), na.rm=TRUE), data.table(variable=factor(2), a=2, b=2)) -test(2182.6, melt(DT.wide, measure.vars=list(b=c("b1","b2"))), data.table(a2=2, variable=factor(1:2), b=c(1,2))) # list yields indices +test(2182.6, melt(DT.wide, measure.vars=list(b=c("b1","b2"))), data.table(a2=2, variable=factor(c("1","2"), b=c(1,2))) # list yields indices # consistency between measure.vars=list with length=1 and length>1 now uses indices for list case, #5209 -test(2182.71, melt(DT.wide, measure.vars=list("a2"), variable.factor=TRUE), data.table(b1=1, b2=2, variable=factor(1L), value=2)) +test(2182.71, melt(DT.wide, measure.vars=list("a2"), variable.factor=TRUE), data.table(b1=1, b2=2, variable=factor(1), value=2)) test(2182.72, melt(DT.wide, measure.vars=c("a2"), variable.factor=TRUE), data.table(b1=1, b2=2, variable=factor("a2"), value=2)) -test(2182.73, melt(DT.wide, measure.vars=list("a2"), variable.factor=FALSE), data.table(b1=1, b2=2, variable=1L, value=2)) +test(2182.73, melt(DT.wide, measure.vars=list("a2"), variable.factor=FALSE), data.table(b1=1, b2=2, variable="1", value=2)) test(2182.74, melt(DT.wide, measure.vars=c("a2"), variable.factor=FALSE), data.table(b1=1, b2=2, variable="a2", value=2)) -test(2182.75, melt(data.table(a=10, b=20), measure.vars=list(n="a"), variable.factor=FALSE), data.table(b=20, variable=1L, n=10))#thanks @mnazarov +test(2182.75, melt(data.table(a=10, b=20), measure.vars=list(n="a"), variable.factor=FALSE), data.table(b=20, variable="1", n=10))#thanks @mnazarov ### First block testing measurev # new variable_table attribute for measure.vars, PR#4731 for multiple issues From c16469bc7b284d28b303f51d5e235a7068110fcb Mon Sep 17 00:00:00 2001 From: Mukul <145585624+Mukulyadav2004@users.noreply.github.com> Date: Tue, 26 Aug 2025 18:13:51 +0530 Subject: [PATCH 10/11] closed bracket --- inst/tests/tests.Rraw | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index 7b08ad63ee..ec2a40392d 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -17417,7 +17417,7 @@ exid = data.table(id=1, expected) test(2182.3, melt(DTid, measure.vars=list(a=c(NA,1), b=2:3), id.vars="id"), exid) test(2182.4, melt(DTid, measure.vars=list(a=c(NA,"a2"), b=c("b1","b2")), id.vars="id"), exid) test(2182.5, melt(DT.wide, measure.vars=list(a=c(NA,1), b=2:3), na.rm=TRUE), data.table(variable=factor(2), a=2, b=2)) -test(2182.6, melt(DT.wide, measure.vars=list(b=c("b1","b2"))), data.table(a2=2, variable=factor(c("1","2"), b=c(1,2))) # list yields indices +test(2182.6, melt(DT.wide, measure.vars=list(b=c("b1","b2"))), data.table(a2=2, variable=factor(c("1","2")), b=c(1,2))) # list yields indices # consistency between measure.vars=list with length=1 and length>1 now uses indices for list case, #5209 test(2182.71, melt(DT.wide, measure.vars=list("a2"), variable.factor=TRUE), data.table(b1=1, b2=2, variable=factor(1), value=2)) test(2182.72, melt(DT.wide, measure.vars=c("a2"), variable.factor=TRUE), data.table(b1=1, b2=2, variable=factor("a2"), value=2)) From f552d561a87dd99c21d5a4bb1dfd34ff7ba39517 Mon Sep 17 00:00:00 2001 From: Mukul <145585624+Mukulyadav2004@users.noreply.github.com> Date: Tue, 26 Aug 2025 21:51:48 +0530 Subject: [PATCH 11/11] EXPECT INTEGER --- inst/tests/tests.Rraw | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index ec2a40392d..fc52054d6c 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -17421,9 +17421,9 @@ test(2182.6, melt(DT.wide, measure.vars=list(b=c("b1","b2"))), data.table(a2=2, # consistency between measure.vars=list with length=1 and length>1 now uses indices for list case, #5209 test(2182.71, melt(DT.wide, measure.vars=list("a2"), variable.factor=TRUE), data.table(b1=1, b2=2, variable=factor(1), value=2)) test(2182.72, melt(DT.wide, measure.vars=c("a2"), variable.factor=TRUE), data.table(b1=1, b2=2, variable=factor("a2"), value=2)) -test(2182.73, melt(DT.wide, measure.vars=list("a2"), variable.factor=FALSE), data.table(b1=1, b2=2, variable="1", value=2)) +test(2182.73, melt(DT.wide, measure.vars=list("a2"), variable.factor=FALSE), data.table(b1=1, b2=2, variable=1L, value=2)) test(2182.74, melt(DT.wide, measure.vars=c("a2"), variable.factor=FALSE), data.table(b1=1, b2=2, variable="a2", value=2)) -test(2182.75, melt(data.table(a=10, b=20), measure.vars=list(n="a"), variable.factor=FALSE), data.table(b=20, variable="1", n=10))#thanks @mnazarov +test(2182.75, melt(data.table(a=10, b=20), measure.vars=list(n="a"), variable.factor=FALSE), data.table(b=20, variable=1L, n=10))#thanks @mnazarov ### First block testing measurev # new variable_table attribute for measure.vars, PR#4731 for multiple issues