Skip to content

Commit 36bd74a

Browse files
Don't warn about numeric coercion in set(j=) (#6595)
* Don't warn about numeric coercion in set(j=) * Fix tests * very conservative about getAttrib()? * also demote condition for 'i' * ???? * undo --------- Co-authored-by: Benjamin Schwendinger <[email protected]>
1 parent 16c6fdc commit 36bd74a

File tree

3 files changed

+10
-5
lines changed

3 files changed

+10
-5
lines changed

NEWS.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,8 @@
122122
123123
5. A GitHub Actions workflow is now in place to warn the entire maintainer team, as well as any contributor following the GitHub repository, when the package is at risk of archival on CRAN [#7008](https://github.com/Rdatatable/data.table/issues/7008). Thanks @tdhock for the original report and @Bisaloo and @TysonStanley for the fix.
124124
125+
6. Using a double vector in `set()`'s `i=` and/or `j=` no longer throws a warning about preferring integer, [#6594](https://github.com/Rdatatable/data.table/issues/6594). While it may improve efficiency to use integer, there's no guarantee it's an improvement and the difference is likely to be minimal. The coercion will still be reported under `datatable.verbose=TRUE`. For package/production use cases, static analyzers such as `lintr::implicit_integer_linter()` can also report when numeric literals should be rewritten as integer literals.
126+
125127
## data.table [v1.17.8](https://github.com/Rdatatable/data.table/milestone/41) (6 July 2025)
126128

127129
1. Internal functions used to signal errors are now marked as non-returning, silencing a compiler warning about potentially unchecked allocation failure. Thanks to Prof. Brian D. Ripley for the report and @aitap for the fix, [#7070](https://github.com/Rdatatable/data.table/pull/7070).

inst/tests/tests.Rraw

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1621,9 +1621,9 @@ test(514, x %chin% y, x %in% y)
16211621

16221622
# Test new function set() in v1.8.0
16231623
DT = data.table(a=1:3,b=4:6)
1624-
test(515, set(DT,2,1,3), data.table(a=c(1L,3L,3L),b=4:6), warning=c("Coerced i from numeric to integer","Coerced j from numeric to integer"))
1624+
test(515, set(DT,2,1,3), data.table(a=c(1L,3L,3L),b=4:6))
16251625
test(516, set(DT,"2",1,3), error="i is type 'character'")
1626-
test(517, set(DT,2L,1,3), DT, warning="Coerced j")
1626+
test(517, options=c(datatable.verbose=TRUE), set(DT,2,1,3), DT, output="Coerced i.*Coerced j")
16271627
# FR #2551 implemented - removed warning from 518
16281628
# test(518, set(DT,2L,1L,3), DT, warning="Coerced 'double' RHS to 'integer'")
16291629
test(518, set(DT,2L,1L,3), DT)

src/assign.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ SEXP setdt_nrows(SEXP x)
219219
if (Rf_inherits(xi, "POSIXlt")) {
220220
error(_("Column %d has class 'POSIXlt'. Please convert it to POSIXct (using as.POSIXct) and run setDT() again. We do not recommend the use of POSIXlt at all because it uses 40 bytes to store one date."), i+1);
221221
}
222-
SEXP dim_xi = getAttrib(xi, R_DimSymbol);
222+
SEXP dim_xi = PROTECT(getAttrib(xi, R_DimSymbol));
223223
R_len_t len_xi, n_dim = length(dim_xi);
224224
if (n_dim) {
225225
if (test_matrix_cols && n_dim > 1) {
@@ -230,6 +230,7 @@ SEXP setdt_nrows(SEXP x)
230230
} else {
231231
len_xi = length(xi);
232232
}
233+
UNPROTECT(1);
233234
if (!base_length) {
234235
base_length = len_xi;
235236
} else if (len_xi != base_length) {
@@ -364,7 +365,8 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values)
364365
} else {
365366
if (isReal(rows)) {
366367
rows = PROTECT(coerceVector(rows, INTSXP)); protecti++;
367-
warning(_("Coerced i from numeric to integer. Please pass integer for efficiency; e.g., 2L rather than 2"));
368+
if (verbose)
369+
Rprintf(_("Coerced %s from numeric to integer. Passing integer directly may be more efficient, e.g., 2L rather than 2"), "i");
368370
}
369371
if (!isInteger(rows))
370372
error(_("i is type '%s'. Must be integer, or numeric is coerced with warning. If i is a logical subset, simply wrap with which(), and take the which() outside the loop if possible for efficiency."), type2char(TYPEOF(rows)));
@@ -417,7 +419,8 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values)
417419
} else {
418420
if (isReal(cols)) {
419421
cols = PROTECT(coerceVector(cols, INTSXP)); protecti++;
420-
warning(_("Coerced j from numeric to integer. Please pass integer for efficiency; e.g., 2L rather than 2"));
422+
if (verbose)
423+
Rprintf(_("Coerced %s from numeric to integer. Passing integer directly may be more efficient, e.g., 2L rather than 2"), "j");
421424
}
422425
if (!isInteger(cols))
423426
error(_("j is type '%s'. Must be integer, character, or numeric is coerced with warning."), type2char(TYPEOF(cols)));

0 commit comments

Comments
 (0)