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.

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

Replace integers with explicit integers (1 -> 1L, etc.) #2573

Merged
merged 19 commits into from
Feb 6, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 5 additions & 5 deletions R/IDateTime.R
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ as.list.IDate <- function(x, ...) NextMethod()
round.IDate <- function (x, digits=c("weeks", "months", "quarters", "years"), ...) {
units <- match.arg(digits)
as.IDate(switch(units,
weeks = round(x, "year") + 7 * (yday(x) %/% 7),
months = ISOdate(year(x), month(x), 1),
quarters = ISOdate(year(x), 3 * (quarter(x)-1) + 1, 1),
years = ISOdate(year(x), 1, 1)))
weeks = round(x, "year") + 7L * (yday(x) %/% 7L),
months = ISOdate(year(x), month(x), 1L),
quarters = ISOdate(year(x), 3L * (quarter(x)-1L) + 1L, 1L),
years = ISOdate(year(x), 1L, 1L)))
}

#Adapted from `+.Date`
Expand All @@ -80,7 +80,7 @@ round.IDate <- function (x, digits=c("weeks", "months", "quarters", "years"), ..
stop("can only subtract from \"IDate\" objects")
if (storage.mode(e1) != "integer")
stop("Internal error: storage mode of IDate is somehow no longer integer")
if (nargs() == 1)
if (nargs() == 1L)
stop("unary - is not defined for \"IDate\" objects")
if (inherits(e2, "difftime"))
stop("difftime objects may not be subtracted from IDate. Use plain integer instead of difftime.")
Expand Down
8 changes: 4 additions & 4 deletions R/as.data.table.R
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ as.data.table.Date <- as.data.table.ITime <- function(x, keep.rownames=FALSE, ..
if (is.matrix(x)) {
return(as.data.table.matrix(x, ...))
}
tt = deparse(substitute(x))[1]
tt = deparse(substitute(x))[1L]
nm = names(x)
# FR #2356 - transfer names of named vector as "rn" column if required
if (!identical(keep.rownames, FALSE) & !is.null(nm))
Expand All @@ -37,7 +37,7 @@ as.data.table.table <- function(x, keep.rownames=FALSE, ...) {
if (is.null(names(val)) || !any(nzchar(names(val))))
setattr(val, 'names', paste("V", rev(seq_along(val)), sep=""))
ans <- data.table(do.call(CJ, c(val, sorted=FALSE)), N = as.vector(x))
setcolorder(ans, c(rev(head(names(ans), -1)), "N"))
setcolorder(ans, c(rev(head(names(ans), -1L)), "N"))
ans
}

Expand Down Expand Up @@ -100,7 +100,7 @@ as.data.table.array <- function(x, keep.rownames=FALSE, sorted=TRUE, value.name=
if (isTRUE(na.rm))
ans = ans[!is.na(N)]
setnames(ans, "N", value.name)
dims = rev(head(names(ans), -1))
dims = rev(head(names(ans), -1L))
setcolorder(ans, c(dims, value.name))
if (isTRUE(sorted))
setkeyv(ans, dims)
Expand Down Expand Up @@ -133,7 +133,7 @@ as.data.table.list <- function(x, keep.rownames=FALSE, ...) {
# Implementing FR #4813 - recycle with warning when nr %% nrows[i] != 0L
if (!n[i] && mn)
warning("Item ", i, " is of size 0 but maximum size is ", mn, ", therefore recycled with 'NA'")
else if (n[i] && mn %% n[i] != 0)
else if (n[i] && mn %% n[i] != 0L)
warning("Item ", i, " is of size ", n[i], " but maximum size is ", mn, " (recycled leaving a remainder of ", mn%%n[i], " items)")
x[[i]] = rep(x[[i]], length.out=mn)
}
Expand Down
14 changes: 7 additions & 7 deletions R/between.R
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ between <- function(x,lower,upper,incbounds=TRUE) {
}

# %between% is vectorised, #534.
"%between%" <- function(x,y) between(x,y[[1]],y[[2]],incbounds=TRUE)
"%between%" <- function(x, y) between(x, y[[1L]], y[[2L]], incbounds=TRUE)
# If we want non inclusive bounds with %between%, just +1 to the left, and -1 to the right (assuming integers)

# issue FR #707
Expand All @@ -22,18 +22,18 @@ inrange <- function(x,lower,upper,incbounds=TRUE) {
subject = setDT(list(l=lower, u=upper))
ops = if (incbounds) c(4L, 2L) else c(5L, 3L) # >=,<= and >,<
verbose = getOption("datatable.verbose")
if (verbose) {last.started.at=proc.time()[3];cat("forderv(query) took ... ");flush.console()}
Copy link
Member Author

@MichaelChirico MichaelChirico Jan 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mattdowle what about two functions, timestart and timetaken? timestart would accept a message as input, cat it, flush.console()s and return the proc.time(); timetaken takes the result of timestart, cats it, and flush.console(), and returns nothing.

(also perhaps this should be done as a separate PR?)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes separate PR. Sometimes there are several nested timing blocks in play. Storing the started.at time in a variable and then passing that (or the appropriate one) to timetaken I think I like the balance of using a common function along with the flexibility of using it in different ways depending on the circumstances. i.e. current timetaken() approach, but actually using it here. IIUC.

if (verbose) {last.started.at=proc.time();cat("forderv(query) took ... ");flush.console()}
xo = forderv(query)
if (verbose) {cat(round(proc.time()[3]-last.started.at,3),"secs\n");flush.console}
ans = bmerge(shallow(subject), query, 1:2, c(1L,1L), FALSE, xo,
0, c(FALSE, TRUE), 0L, "all", ops, integer(0),
if (verbose) {cat(timetaken(last.started.at),"\n"); flush.console()}
ans = bmerge(shallow(subject), query, 1L:2L, c(1L,1L), FALSE, xo,
0, c(FALSE, TRUE), 0L, "all", ops, integer(0L),
1L, verbose) # fix for #1819, turn on verbose messages
options(datatable.verbose=FALSE)
setDT(ans[c("starts", "lens")], key=c("starts", "lens"))
options(datatable.verbose=verbose)
if (verbose) {last.started.at=proc.time()[3];cat("Generating final logical vector ... ");flush.console()}
if (verbose) {last.started.at=proc.time();cat("Generating final logical vector ... ");flush.console()}
.Call(Cinrange, idx <- vector("logical", length(x)), xo, ans[["starts"]], ans[["lens"]])
if (verbose) {cat("done in",round(proc.time()[3]-last.started.at,3),"secs\n");flush.console}
if (verbose) {cat("done in",timetaken(last.started.at),"\n"); flush.console}
idx
}

Expand Down
5 changes: 2 additions & 3 deletions R/bmerge.R
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,10 @@ bmerge <- function(i, x, leftcols, rightcols, io, xo, roll, rollends, nomatch, m
set(i, j=lc, value=newval)
}
}
if (verbose) {last.started.at=proc.time()[3];cat("Starting bmerge ...");flush.console()}
if (verbose) {last.started.at=proc.time();cat("Starting bmerge ...");flush.console()}
ans = .Call(Cbmerge, i, x, as.integer(leftcols), as.integer(rightcols), io<-haskey(i), xo, roll, rollends, nomatch, mult, ops, nqgrp, nqmaxgrp)
# NB: io<-haskey(i) necessary for test 579 where the := above change the factor to character and remove i's key
if (verbose) {cat("done in",round(proc.time()[3]-last.started.at,3),"secs\n");flush.console()}
if (verbose) {cat("done in",timetaken(last.started.at),"\n"); flush.console()}

# in the caller's shallow copy, see comment at the top of this function for usage
# We want to leave the coercions to i in place otherwise, since the caller depends on that to build the result
Expand All @@ -105,4 +105,3 @@ bmerge <- function(i, x, leftcols, rightcols, io, xo, roll, rollends, nomatch, m
return(ans)
}


Loading