Skip to content

Commit 331f5db

Browse files
remove more explicit returns
1 parent f78044e commit 331f5db

File tree

6 files changed

+72
-80
lines changed

6 files changed

+72
-80
lines changed

R/data.table.R

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1939,7 +1939,7 @@ replace_dot_alias = function(e) {
19391939
if (inherits(x, 'data.table')) .Call(C_unlock, x)
19401940
else return(lapply(x, runlock, current_depth = current_depth + 1L))
19411941
}
1942-
return(invisible())
1942+
invisible()
19431943
}
19441944
runlock(ans)
19451945
if (verbose) {cat(timetaken(last.started.at),"\n"); flush.console()}
@@ -3118,7 +3118,7 @@ is_constantish = function(q, check_singleton=FALSE) {
31183118
return(FALSE)
31193119
}
31203120
# calls are allowed <=> there's no SYMBOLs in the sub-AST
3121-
return(length(all.vars(q, max.names=1L, unique=FALSE)) == 0L)
3121+
length(all.vars(q, max.names=1L, unique=FALSE)) == 0L
31223122
}
31233123
.gshift_ok = function(q) {
31243124
q = match.call(shift, q)
@@ -3192,7 +3192,7 @@ is_constantish = function(q, check_singleton=FALSE) {
31923192
stopf("It looks like you re-used `:=` in argument %d a functional assignment call -- use `=` instead: %s(col1=val1, col2=val2, ...)", jj-1L, call_name)
31933193
}
31943194

3195-
.prepareFastSubset = function(isub, x, enclos, notjoin, verbose = FALSE){
3195+
.prepareFastSubset = function(isub, x, enclos, notjoin, verbose=FALSE) {
31963196
## helper that decides, whether a fast binary search can be performed, if i is a call
31973197
## For details on the supported queries, see \code{\link{datatable-optimize}}
31983198
## Additional restrictions are imposed if x is .SD, or if options indicate that no optimization
@@ -3342,14 +3342,9 @@ is_constantish = function(q, check_singleton=FALSE) {
33423342
setkeyv(i, idxCols)
33433343
on = on[idxCols] ## make sure 'on' is in the correct order. Otherwise the logic won't recognise that a key / index already exists.
33443344
}
3345-
return(list(i = i,
3346-
on = on,
3347-
notjoin = notjoin
3348-
)
3349-
)
3345+
list(i=i, on=on, notjoin=notjoin)
33503346
}
33513347

3352-
33533348
.parse_on = function(onsub, isnull_inames) {
33543349
## helper that takes the 'on' string(s) and extracts comparison operators and column names from it.
33553350
#' @param onsub the substituted on

R/fcast.R

Lines changed: 38 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -214,42 +214,42 @@ dcast.data.table = function(data, formula, fun.aggregate = NULL, sep = "_", ...,
214214
# 'dat' != 'data'? then setkey to speed things up (slightly), else ad-hoc (for now). Still very fast!
215215
if (!is.null(fun.call) || !is.null(subset))
216216
setkeyv(dat, varnames)
217-
if (length(rhsnames)) {
218-
lhs = shallow(dat, lhsnames); rhs = shallow(dat, rhsnames); val = shallow(dat, valnames)
219-
# handle drop=TRUE/FALSE - Update: Logic moved to R, AND faster than previous version. Take that... old me :-).
220-
if (all(drop)) {
221-
map = setDT(lapply(list(lhsnames, rhsnames), function(cols) frankv(dat, cols=cols, ties.method="dense", na.last=FALSE))) # #2202 fix
222-
maporder = lapply(map, order_)
223-
mapunique = lapply(seq_along(map), function(i) .Call(CsubsetVector, map[[i]], maporder[[i]]))
224-
lhs = .Call(CsubsetDT, lhs, maporder[[1L]], seq_along(lhs))
225-
rhs = .Call(CsubsetDT, rhs, maporder[[2L]], seq_along(rhs))
226-
} else {
227-
lhs_ = if (!drop[1L]) cj_uniq(lhs) else setkey(unique(lhs, by=names(lhs)))
228-
rhs_ = if (!drop[2L]) cj_uniq(rhs) else setkey(unique(rhs, by=names(rhs)))
229-
map = vector("list", 2L)
230-
.Call(Csetlistelt, map, 1L, lhs_[lhs, which=TRUE])
231-
.Call(Csetlistelt, map, 2L, rhs_[rhs, which=TRUE])
232-
setDT(map)
233-
mapunique = vector("list", 2L)
234-
.Call(Csetlistelt, mapunique, 1L, seq_len(nrow(lhs_)))
235-
.Call(Csetlistelt, mapunique, 2L, seq_len(nrow(rhs_)))
236-
lhs = lhs_; rhs = rhs_
237-
}
238-
maplen = lengths(mapunique)
239-
idx = do.call(CJ, mapunique)[map, 'I' := .I][["I"]] # TO DO: move this to C and avoid materialising the Cross Join.
240-
some_fill = anyNA(idx)
241-
fill.default = if (run_agg_funs && is.null(fill) && some_fill) dat_for_default_fill[, maybe_err(eval(fun.call))]
242-
if (run_agg_funs && is.null(fill) && some_fill) {
243-
fill.default = dat_for_default_fill[0L][, maybe_err(eval(fun.call))]
244-
}
245-
ans = .Call(Cfcast, lhs, val, maplen[[1L]], maplen[[2L]], idx, fill, fill.default, is.null(fun.call), some_fill)
246-
allcols = do.call(paste, c(rhs, sep=sep))
247-
if (length(valnames) > 1L)
248-
allcols = do.call(paste, if (identical(".", allcols)) list(valnames, sep=sep)
249-
else c(CJ(valnames, allcols, sorted=FALSE), sep=sep))
250-
# removed 'setcolorder()' here, #1153
251-
setattr(ans, 'names', c(lhsnames, allcols))
252-
setDT(ans); setattr(ans, 'sorted', lhsnames)
253-
} else internal_error("empty rhsnames") # nocov
254-
return(ans)
217+
if (!length(rhsnames)) internal_error("empty rhsnames") # nocov
218+
lhs = shallow(dat, lhsnames); rhs = shallow(dat, rhsnames); val = shallow(dat, valnames)
219+
# handle drop=TRUE/FALSE - Update: Logic moved to R, AND faster than previous version. Take that... old me :-).
220+
if (all(drop)) {
221+
map = setDT(lapply(list(lhsnames, rhsnames), function(cols) frankv(dat, cols=cols, ties.method="dense", na.last=FALSE))) # #2202 fix
222+
maporder = lapply(map, order_)
223+
mapunique = lapply(seq_along(map), function(i) .Call(CsubsetVector, map[[i]], maporder[[i]]))
224+
lhs = .Call(CsubsetDT, lhs, maporder[[1L]], seq_along(lhs))
225+
rhs = .Call(CsubsetDT, rhs, maporder[[2L]], seq_along(rhs))
226+
} else {
227+
lhs_ = if (!drop[1L]) cj_uniq(lhs) else setkey(unique(lhs, by=names(lhs)))
228+
rhs_ = if (!drop[2L]) cj_uniq(rhs) else setkey(unique(rhs, by=names(rhs)))
229+
map = vector("list", 2L)
230+
.Call(Csetlistelt, map, 1L, lhs_[lhs, which=TRUE])
231+
.Call(Csetlistelt, map, 2L, rhs_[rhs, which=TRUE])
232+
setDT(map)
233+
mapunique = vector("list", 2L)
234+
.Call(Csetlistelt, mapunique, 1L, seq_len(nrow(lhs_)))
235+
.Call(Csetlistelt, mapunique, 2L, seq_len(nrow(rhs_)))
236+
lhs = lhs_; rhs = rhs_
237+
}
238+
maplen = lengths(mapunique)
239+
idx = do.call(CJ, mapunique)[map, 'I' := .I][["I"]] # TO DO: move this to C and avoid materialising the Cross Join.
240+
some_fill = anyNA(idx)
241+
fill.default = if (run_agg_funs && is.null(fill) && some_fill) dat_for_default_fill[, maybe_err(eval(fun.call))]
242+
if (run_agg_funs && is.null(fill) && some_fill) {
243+
fill.default = dat_for_default_fill[0L][, maybe_err(eval(fun.call))]
244+
}
245+
ans = .Call(Cfcast, lhs, val, maplen[[1L]], maplen[[2L]], idx, fill, fill.default, is.null(fun.call), some_fill)
246+
allcols = do.call(paste, c(rhs, sep=sep))
247+
if (length(valnames) > 1L)
248+
allcols = do.call(paste, if (identical(".", allcols)) list(valnames, sep=sep)
249+
else c(CJ(valnames, allcols, sorted=FALSE), sep=sep))
250+
# removed 'setcolorder()' here, #1153
251+
setattr(ans, 'names', c(lhsnames, allcols))
252+
setDT(ans)
253+
setattr(ans, 'sorted', lhsnames)
254+
ans
255255
}

R/fdroplevels.R

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ fdroplevels = function(x, exclude = if (anyNA(levels(x))) NULL else NA, ...) {
55
ans = match(as.integer(x), lev)
66
setattr(ans, 'levels', levels(x)[lev])
77
setattr(ans, 'class', class(x))
8-
return(ans)
8+
ans
99
}
1010

1111
droplevels.data.table = function(x, except=NULL, exclude, in.place=NULL, ...){

R/foverlaps.R

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -181,23 +181,22 @@ foverlaps = function(x, y, by.x=if (!is.null(key(x))) key(x) else key(y), by.y=k
181181
if (which) {
182182
if (mult %chin% c("first", "last"))
183183
return(olaps$yid)
184-
else if (!is.na(nomatch))
185-
return(.Call(CsubsetDT, olaps, which(olaps$yid > 0L), seq_along(olaps)))
186-
else return(olaps)
187-
} else {
188184
if (!is.na(nomatch))
189-
olaps = .Call(CsubsetDT, olaps, which(olaps$yid > 0L), seq_along(olaps))
190-
ycols = setdiff(names(origy), head(by.y, -2L))
191-
idx = chmatch(ycols, names(origx), nomatch=0L)
192-
ans = .Call(CsubsetDT, origx, olaps$xid, seq_along(origx))
193-
if (any(idx>0L))
194-
setnames(ans, names(ans)[idx], paste0("i.", names(ans)[idx]))
195-
xcols1 = head(by.x, -2L)
196-
xcols2 = setdiff(names(ans), xcols1)
197-
ans[, (ycols) := .Call(CsubsetDT, origy, olaps$yid, chmatch(ycols, names(origy)))]
198-
setcolorder(ans, c(xcols1, ycols, xcols2))
199-
return(ans[])
185+
return(.Call(CsubsetDT, olaps, which(olaps$yid > 0L), seq_along(olaps)))
186+
return(olaps)
200187
}
188+
if (!is.na(nomatch))
189+
olaps = .Call(CsubsetDT, olaps, which(olaps$yid > 0L), seq_along(olaps))
190+
ycols = setdiff(names(origy), head(by.y, -2L))
191+
idx = chmatch(ycols, names(origx), nomatch=0L)
192+
ans = .Call(CsubsetDT, origx, olaps$xid, seq_along(origx))
193+
if (any(idx > 0L))
194+
setnames(ans, names(ans)[idx], paste0("i.", names(ans)[idx]))
195+
xcols1 = head(by.x, -2L)
196+
xcols2 = setdiff(names(ans), xcols1)
197+
ans[, (ycols) := .Call(CsubsetDT, origy, olaps$yid, chmatch(ycols, names(origy)))]
198+
setcolorder(ans, c(xcols1, ycols, xcols2))
199+
ans[]
201200
}
202201

203202
# Notes: (If there's a better way than the solution I propose here, I'd be glad to apply it.)

R/setkey.R

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ setindex = function(...) setkey(..., physical=FALSE)
1212
setindexv = function(x, cols, verbose=getOption("datatable.verbose")) {
1313
if (is.list(cols)) {
1414
sapply(cols, setkeyv, x=x, verbose=verbose, physical=FALSE)
15-
return(invisible(x))
15+
invisible(x)
1616
} else {
1717
setkeyv(x, cols, verbose=verbose, physical=FALSE)
1818
}
@@ -207,23 +207,21 @@ forder = function(..., na.last=TRUE, decreasing=FALSE, method="radix")
207207
fsort = function(x, decreasing=FALSE, na.last=FALSE, internal=FALSE, verbose=FALSE, ...)
208208
{
209209
containsNAs = FALSE
210-
if (typeof(x)=="double" && !decreasing && !(containsNAs <- anyNA(x))) {
211-
if (internal) stopf("Internal code should not be being called on type double")
212-
return(.Call(Cfsort, x, verbose))
210+
if (typeof(x) == "double" && !decreasing && !(containsNAs <- anyNA(x))) {
211+
if (internal) stopf("Internal code should not be being called on type double")
212+
return(.Call(Cfsort, x, verbose))
213213
}
214-
else {
215-
# fsort is now exported for testing. Trying to head off complaints "it's slow on integer"
216-
# The only places internally we use fsort internally (3 calls, all on integer) have had internal=TRUE added for now.
217-
# TODO: implement integer and character in Cfsort and remove this branch and warning
218-
if (!internal){
219-
if (typeof(x)!="double") warningf("Input is not a vector of type double. New parallel sort has only been done for double vectors so far. Using one thread.")
220-
if (decreasing) warningf("New parallel sort has not been implemented for decreasing=TRUE so far. Using one thread.")
221-
if (containsNAs) warningf("New parallel sort has not been implemented for vectors containing NA values so far. Using one thread.")
222-
}
223-
orderArg = if (decreasing) -1L else 1L
224-
o = forderv(x, order=orderArg, na.last=na.last)
225-
return( if (length(o)) x[o] else x )
214+
# fsort is now exported for testing. Trying to head off complaints "it's slow on integer"
215+
# The only places internally we use fsort internally (3 calls, all on integer) have had internal=TRUE added for now.
216+
# TODO: implement integer and character in Cfsort and remove this branch and warning
217+
if (!internal) {
218+
if (typeof(x) != "double") warningf("Input is not a vector of type double. New parallel sort has only been done for double vectors so far. Using one thread.")
219+
if (decreasing) warningf("New parallel sort has not been implemented for decreasing=TRUE so far. Using one thread.")
220+
if (containsNAs) warningf("New parallel sort has not been implemented for vectors containing NA values so far. Using one thread.")
226221
}
222+
orderArg = if (decreasing) -1L else 1L
223+
o = forderv(x, order=orderArg, na.last=na.last)
224+
if (length(o)) x[o] else x
227225
}
228226

229227
setorder = function(x, ..., na.last=FALSE)

R/xts.R

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,6 @@ as.xts.data.table = function(x, numeric.only = TRUE, ...) {
2525
if (!all(colsNumeric)) warningf("Following columns are not numeric and will be omitted: %s", brackify(names(colsNumeric)[!colsNumeric]))
2626
r = r[, .SD, .SDcols = names(colsNumeric)[colsNumeric]]
2727
}
28-
return(xts::xts(as.matrix(r), order.by = if (inherits(x[[1L]], "IDate")) as.Date(x[[1L]]) else x[[1L]]))
28+
xts::xts(as.matrix(r), order.by = if (inherits(x[[1L]], "IDate")) as.Date(x[[1L]]) else x[[1L]])
2929
}
3030
# nocov end

0 commit comments

Comments
 (0)