Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't warn about numeric coercion in set(j=) #6595

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Don't warn about numeric coercion in set(j=) #6595

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cd7e004
Don't warn about numeric coercion in set(j=)
MichaelChirico Oct 31, 2024
1aeb6ae
Fix tests
MichaelChirico Oct 31, 2024
ffe75b2
very conservative about getAttrib()?
MichaelChirico Oct 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,8 @@ rowwiseDT(

8. `measurev()` was implemented and documented in v1.15.0, for use within `melt()`, and it is now exported (dependent packages can now use without a NOTE from CRAN check).

9. Using a double vector in `set()`'s `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.

# data.table [v1.16.0](https://github.com/Rdatatable/data.table/milestone/30) (25 August 2024)

## BREAKING CHANGES
Expand Down
4 changes: 2 additions & 2 deletions inst/tests/tests.Rraw
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1648,9 +1648,9 @@ test(514, x %chin% y, x %in% y)

# Test new function set() in v1.8.0
DT = data.table(a=1:3,b=4:6)
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"))
test(515, set(DT,2,1,3), data.table(a=c(1L,3L,3L),b=4:6), warning="Coerced i from numeric to integer")
test(516, set(DT,"2",1,3), error="i is type 'character'")
test(517, set(DT,2L,1,3), DT, warning="Coerced j")
test(517, options=c(datatable.verbose=TRUE), set(DT,2L,1,3), DT, output="Coerced j")
# FR #2551 implemented - removed warning from 518
# test(518, set(DT,2L,1L,3), DT, warning="Coerced 'double' RHS to 'integer'")
test(518, set(DT,2L,1L,3), DT)
Expand Down
6 changes: 4 additions & 2 deletions src/assign.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ SEXP setdt_nrows(SEXP x)
if (Rf_inherits(xi, "POSIXlt")) {
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);
}
SEXP dim_xi = getAttrib(xi, R_DimSymbol);
SEXP dim_xi = PROTECT(getAttrib(xi, R_DimSymbol));
R_len_t len_xi;
// NB: LENGTH() produces an undefined large number here on R 3.3.0.
// There's also a note in NEWS for R 3.1.0 saying length() should always be used by packages,
Expand All @@ -234,6 +234,7 @@ SEXP setdt_nrows(SEXP x)
} else {
len_xi = LENGTH(xi);
}
UNPROTECT(1);
if (!base_length) {
base_length = len_xi;
} else if (len_xi != base_length) {
Expand Down Expand Up @@ -425,7 +426,8 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values)
} else {
if (isReal(cols)) {
cols = PROTECT(coerceVector(cols, INTSXP)); protecti++;
warning(_("Coerced j from numeric to integer. Please pass integer for efficiency; e.g., 2L rather than 2"));
if (verbose)
Rprintf(_("Coerced j from numeric to integer. Please pass integer for efficiency; e.g., 2L rather than 2"));
}
if (!isInteger(cols))
error(_("j is type '%s'. Must be integer, character, or numeric is coerced with warning."), type2char(TYPEOF(cols)));
Expand Down
Loading