From 21caf0d69c7ef2221b4347dc4221b3781bffa0c9 Mon Sep 17 00:00:00 2001 From: venom1204 Date: Sun, 9 Aug 2026 19:55:19 +0000 Subject: [PATCH] updated changews --- NEWS.md | 2 ++ R/data.table.R | 5 +++++ inst/tests/tests.Rraw | 12 ++++++++++++ src/assign.c | 10 +++++++++- 4 files changed, 28 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index b875f7dc26..1d791723de 100644 --- a/NEWS.md +++ b/NEWS.md @@ -46,6 +46,8 @@ 12. `frank()` gains an `order` argument (matching `frankv()`) and now intercepts the unary minus symbol (e.g., `frank(-dates)`) to support reverse ranking even for types where unary `-` is not defined in R, such as `Date` or `character` vectors, [#5489](https://github.com/Rdatatable/data.table/issues/5489). Thanks @hope-data-science for the request and @venom1204 for the implementation. +13. Type overrides (e.g. changing an integer column to character) are now permitted in `:=` when `i` is a call to `order()` or `forder()`, [#2925](https://github.com/Rdatatable/data.table/issues/2925). Previously, any non-missing `i` forced `data.table` into "subset update" mode which prevents type changes; this restriction is now relaxed for ordering calls since they encompass all rows. Thanks @MichaelChirico for the report and @venom1204 for the fix. + ### BUG FIXES 1. `fread()` with `skip=0` and `(header=TRUE|FALSE)` no longer skips the first row when it has fewer fields than subsequent rows, [#7463](https://github.com/Rdatatable/data.table/issues/7463). Thanks @emayerhofer for the report and @ben-schwen for the fix. diff --git a/R/data.table.R b/R/data.table.R index 7ac2f69917..56b45c1404 100644 --- a/R/data.table.R +++ b/R/data.table.R @@ -548,6 +548,7 @@ replace_dot_alias = function(e) { on.exit(options(oldverbose)) } .global$print="" + i_is_order = FALSE missingby = missing(by) && missing(keyby) # for tests 359 & 590 where passing by=NULL results in data.table not vector if (missingby || missing(j)) { if (!missingby) warningf("Ignoring by/keyby because 'j' is not supplied") @@ -727,6 +728,7 @@ replace_dot_alias = function(e) { } if (!missing(i)) { xo = NULL + i_is_order = is.call(isub) && any(as.character(isub[[1L]]) %chin% c("order", "forder")) if (identical(isub, NA)) { # only possibility *isub* can be NA (logical) is the symbol NA itself; i.e. DT[NA] # replace NA in this case with NA_integer_ as that's almost surely what user intended to @@ -1839,6 +1841,9 @@ replace_dot_alias = function(e) { } } # TODO?: use set() here now that it can add new columns. Then remove newnames and alloc logic above. + if (isTRUE(i_is_order) && !is.null(irows)) { + setattr(irows, ".datatable.full_order", TRUE) + } .Call(Cassign,x,irows,cols,newnames,jval) return(suppPrint(x)) } diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index f9f95f5722..0a0ba85f41 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -21974,3 +21974,15 @@ test(2386.10, frank(-dates, order=-1L), frankv(dates, order=-1L), warning=warn) test(2386.11, frank(+dates), frankv(dates, order=1L)) chars = c("b", "a", "c", "a") test(2386.12, frank(-chars), frankv(chars, order=-1L)) + +# #2925: Allow type override in := when i is order() +DT = data.table(a = 1:10) +DT[order(-a), a := paste0(a)] +test(2925.1, class(DT$a), "character") +test(2925.2, DT$a, as.character(1:10)) +DT = data.table(a = rep(letters[1:3], 3), b = 1:9) +DT[order(-b), a := factor(a, levels = unique(a))] +test(2925.3, is.factor(DT$a), TRUE) +test(2925.4, levels(DT$a), c("c", "b", "a")) +DT = data.table(a = 1:10) +test(2925.5, DT[1:5, a := as.character(a)], data.table(a=1:10), warning="Coercing 'character' RHS to 'integer'") diff --git a/src/assign.c b/src/assign.c index 57f0e6eb9b..44c1f35e27 100644 --- a/src/assign.c +++ b/src/assign.c @@ -363,6 +363,8 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values) bool verbose=GetVerbose(); int ndelete=0; // how many columns are being deleted int *buf; + SEXP full_order_attr = (isNull(rows)) ? R_NilValue : getAttrib(rows, install(".datatable.full_order")); + bool is_full_order = (isLogical(full_order_attr) && length(full_order_attr)>0 && LOGICAL(full_order_attr)[0]==1); if (isNull(dt)) error(_("assign has been passed a NULL dt")); if (TYPEOF(dt) != VECSXP) error(_("dt passed to %s isn't type VECSXP"), "assign"); if (islocked(dt)) @@ -602,14 +604,20 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values) if (coln+1 > oldncol) { // new column SET_VECTOR_ELT(dt, coln, targetcol=allocNAVectorLike(thisvalue, nrow)); + if (isVectorAtomic(thisvalue)) copyMostAttrib(thisvalue,targetcol); // initialize with NAs for when 'rows' is a subset and it doesn't touch // do not try to save the time to NA fill (contiguous branch free assign anyway) since being // sure all items will be written to (isNull(rows), length(rows), vlen<1, targetlen) is not worth the risk. - if (isVectorAtomic(thisvalue)) copyMostAttrib(thisvalue,targetcol); // class etc but not names // else for lists (such as data.frame and data.table) treat them as raw lists and drop attribs if (vlen<1) continue; // e.g. DT[,newcol:=integer()] (adding new empty column) } else { // existing column targetcol = VECTOR_ELT(dt,coln); + if (is_full_order && !isNull(rows) && (TYPEOF(targetcol) != TYPEOF(thisvalue) || isFactor(thisvalue))) { + targetcol = PROTECT(allocNAVectorLike(thisvalue, nrow)); + if (isVectorAtomic(thisvalue)) copyMostAttrib(thisvalue, targetcol); + SET_VECTOR_ELT(dt, coln, targetcol); + UNPROTECT(1); + } } const char *ret = memrecycle(targetcol, rows, 0, targetlen, thisvalue, 0, -1, coln+1, CHAR(STRING_ELT(names, coln))); if (ret) warning("%s", ret); // # notranslate